Re: [Qemu-devel] [PATCH v7 10/20] hw/arm/smmuv3: Implement translate callback

[Auger Eric <eric.auger@redhat.com>]

Hi Peter,
On 06/02/18 13:43, Peter Maydell wrote:
> On 6 February 2018 at 12:19, Auger Eric <eric.auger@redhat.com> wrote:
>> On 09/10/17 19:45, Peter Maydell wrote:
>>> Incidentally, the spec requires us to perform memory accesses as
>>> at least 64-bit single-copy atomic (see 3.21.3) -- does this do that?
>>> (This gets important with SMP when the guest on another CPU might
>>> be updating the STE or page table entry at the same time as we're
>>> reading it...)
>>
>> Among all your comments on v7, here is the one I am the least
>> comfortable with. I was not able to figure out whether
>> dma_memory_read(), which I use now for all the descriptor accesses
>> guarantees this 64b single copy atomicity.
> 
> It doesn't -- it winds up in flatview_read_continue(), which will
> do a memcpy() from guest ram into the buffer, and since it's
> using an arbitrary passed-in length value the chances are high
> it won't end up using an atomic access on the host.
> 
> This is a nasty issue which we haven't figured out at all yet;
> see also this thread:
> https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03067.html
> 
> For the moment my suggestion would be that when you need
> to do guest memory accesses that have atomicity requirements you:
>  (a) use an accessor function which specifically loads a
>   quantity of the correct size, rather than one which takes
>   an arbitrary start-and-length
>   (when we add APIs which do have atomicity guarantees they'll
>   be "load 64 bit word" etc, so using our existing APIs of
>   that form should avoid needing to restructure this code later)
>  (b) add a TODO comment noting the required atomicity

OK thank you for the link. To start with, I will add a comment in next
version then.

Thanks

Eric
> 
> thanks
> -- PMM
>