[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH V1 14/16] xen/ioreq: Use guest_cmpxchg64() instead of cmpxchg()




On 23.09.20 21:12, Julien Grall wrote:

Hi Julien




On 16/09/2020 10:04, Jan Beulich wrote:
On 10.09.2020 22:22, Oleksandr Tyshchenko wrote:
@@ -1325,7 +1327,7 @@ static int hvm_send_buffered_ioreq(struct hvm_ioreq_server *s, ioreq_t *p)

           new.read_pointer = old.read_pointer - n * IOREQ_BUFFER_SLOT_NUM;            new.write_pointer = old.write_pointer - n * IOREQ_BUFFER_SLOT_NUM;
-        cmpxchg(&pg->ptrs.full, old.full, new.full);
+        guest_cmpxchg64(d, &pg->ptrs.full, old.full, new.full);

But the memory we're updating is shared with s->emulator, not with d,
if I'm not mistaken.

It is unfortunately shared with both s->emulator and d when using the
legacy interface.

When using magic pages they should be punched out of the P2M by the time the code gets here, so the memory should not be guest-visible.

Can you point me to the code that doing this?

Cheers,

If we are not going to use legacy interface on Arm we will have a page to be mapped in a single domain at the time.
Right, but this is common code... You have to think what would be the implication if we are using the legacy interface.

Thankfully the only user of the legacy interface is x86 so far and there is not concern regarding the atomics operations.

If we are happy to consider that the legacy interface will never be used (I am starting to be worry that one will ask it on Arm...) then we should be fine.

I think would be worth documenting in the commit message and the code (hvm_allow_set_param()) that the legacy interface *must* not be used without revisiting the code.

Will do.


--
Regards,

Oleksandr Tyshchenko




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.