Re: [PATCH 1/2] xen/arm: Convert runstate address during hypcall

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 12 Jun 2020, Julien Grall wrote:
> > Writing byte by byte is a different case. That is OK. In that case, the
> > guest could see the state after 3 bytes written and it would be fine and
> > consistent.
> 
> Why? What does actually prevent a guest to see an in-between value?
> 
> To give a concrete example, if the original value is 0xabc and you want to
> write 0xdef. Why would the guest never see 0xabf or 0xaec?

That is a good question, but the minimum granularity is 1 byte, so if

  current: 0xaabbcc
  new: 0xddeeff

Can the guest see one of the following?

  0xaabbff
  0xaaeecc

Yes, it can. I don't think it is a problem in this case because we only
change 1 byte, so to continue with the example:

  current: 0xaabbcc
  new: 0xffbbcc

So the only values the VM can see are either 0xaabbcc or 0xffbbcc.


> > If this hadn't been the case, then yes, the existing code
> > would also be buggy.
> > 
> > So if we did the write with a memcpy, it would be fine, no need for
> > atomics:
> > 
> >    memcpy(&guest_runstate->state_entry_time,
> >           &v->runstate.state_entry_time,
> >           XXX);
> > 
> > 
> > The |= case is different: GCC could implement it in any way it likes,
> > including going through a zero-write to any of the bytes in the word, or
> > doing an addition then a subtraction. GCC doesn't make any guarantees.
> > If we want guarantees we need to use atomics.
> 
> Yes GCC can generate assembly for |= in any way it likes. But so does for
> memcpy(). So I still don't understand why one would be fine for you and not
> the other...

It is down to the implementation of memcpy to guarantee that the only
thing memcpy does is memory copies. memcpy doesn't specify whether it is
going to use byte-copies or word-copies, but it should only do copies.
If we have to write memcpy in assembly to make it so, so be it :-)