Hi,
I have been trying to understand the Arm’s FFA by reading Hafnium’s implementation, and recently discovered an inconsistency between the specification and the implementation regarding the sender’s mode of donated pages after an invocation of FFA_MEM_DONATE.
According to item 12.1 of 11.1.1.2 section in https://developer.arm.com/documentation/den0077/latest,
12. If the call executes successfully, the Relayer must:
1. Ensure that the state of the memory region in the participating FF-A components is observed as follows:
1. If the Receiver is a PE endpoint or a SEPID associated with a dependent peripheral device, then:
• Owner-NA for the Owner.
• !Owner-NA for the Receiver.
the sender(owner) is supposed to still own the pages, which is not the case in the implementation(https://git.trustedfirmware.org/hafnium/hafnium.git/tree/src… ).
I don’t know if there is a reason behind this, but I think at least a consequence of it is that the sender would be not able to invoke FFA_MEM_RECLAIM successfully before the receiver retrieves those pages, which implies possible “page leakage” if the receiver refuses to cooperate.
I also wrote a simple test case in which the sender invokes FFA_MEM_RECLAIM right after FFA_MEM_DONATE. As expected, Hafnium returned FFA_DENIED(https://git.trustedfirmware.org/hafnium/hafnium.git/tree/src/ffa… ).
If it is indeed a bug, then a simple fix could be removing “| MM_MODE_UNOWNED” at line 572.
Regards,
Zongyuan Liu
On Mon, 4 Jan 2021 at 17:03, Raghu Krishnamurthy via Hafnium <
hafnium(a)lists.trustedfirmware.org> wrote:
> Hi All,
>
> I was going through src/mm.c in the hafnium repo and noticed a couple of
> things that I think are problem:
>
> 1. mm_vm_defrag and mm_defrag, which eventually call mm_ptable_defrag
> appears to be changing page entries into block entries. Per D5.10.1 of ARM
> ARM, this should use break-before-make, but I don't see it being done in
> code. Am I missing where this is done in code, or is there some reason
> break-before-make is not required during defrag(that is not obvious to me
> from code or comments)?
> 2. In mm_replace_entry, line 278
> (https://git.trustedfirmware.org/hafnium/hafnium.git/tree/src/mm.c#n278)
> is
> not followed by a DSB + ISB as required by the break-before-make. Without
> the DSB, a load/store that uses a VA translated by the PTE being replaced
> could be reordered before line 278 and fault. Once again, this looks like a
> bug to me but is there some reason that is not obvious due to which it does
> not require barriers?
>
It looks like that code was added in
https://hafnium-review.googlesource.com/c/hafnium/+/3260. Maybe Andrew
Scull or Wedson can comment, if they can remember?
