* [Xen-devel] [PATCH] x86/HVM: use single (atomic) MOV for aligned emulated writes
@ 2019-09-16 9:40 Jan Beulich
2019-12-20 16:23 ` [Xen-devel] Ping: " Jan Beulich
0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2019-09-16 9:40 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Roger Pau Monné
Using memcpy() may result in multiple individual byte accesses
(dependening how memcpy() is implemented and how the resulting insns,
e.g. REP MOVSB, get carried out in hardware), which isn't what we
want/need for carrying out guest insns as correctly as possible. Fall
back to memcpy() only for accesses not 2, 4, or 8 bytes in size.
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
TBD: Besides it still being open whether the linear_write() path also
needs playing with), a question also continues to be whether we'd
want to extend this to reads as well. linear_{read,write}()
currently don't use hvmemul_map_linear_addr(), i.e. in both cases
I'd need to also fiddle with __hvm_copy() (perhaps by making the
construct below a helper function).
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -1324,7 +1324,14 @@ static int hvmemul_write(
if ( !mapping )
return linear_write(addr, bytes, p_data, pfec, hvmemul_ctxt);
- memcpy(mapping, p_data, bytes);
+ /* Where possible use single (and hence generally atomic) MOV insns. */
+ switch ( bytes )
+ {
+ case 2: write_u16_atomic(mapping, *(uint16_t *)p_data); break;
+ case 4: write_u32_atomic(mapping, *(uint32_t *)p_data); break;
+ case 8: write_u64_atomic(mapping, *(uint64_t *)p_data); break;
+ default: memcpy(mapping, p_data, bytes); break;
+ }
hvmemul_unmap_linear_addr(mapping, addr, bytes, hvmemul_ctxt);
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Xen-devel] Ping: [PATCH] x86/HVM: use single (atomic) MOV for aligned emulated writes
2019-09-16 9:40 [Xen-devel] [PATCH] x86/HVM: use single (atomic) MOV for aligned emulated writes Jan Beulich
@ 2019-12-20 16:23 ` Jan Beulich
2019-12-27 16:06 ` Andrew Cooper
0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2019-12-20 16:23 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Wei Liu, Roger Pau Monné
On 16.09.2019 11:40, Jan Beulich wrote:
> Using memcpy() may result in multiple individual byte accesses
> (dependening how memcpy() is implemented and how the resulting insns,
> e.g. REP MOVSB, get carried out in hardware), which isn't what we
> want/need for carrying out guest insns as correctly as possible. Fall
> back to memcpy() only for accesses not 2, 4, or 8 bytes in size.
>
> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> TBD: Besides it still being open whether the linear_write() path also
> needs playing with, a question also continues to be whether we'd
> want to extend this to reads as well. linear_{read,write}()
> currently don't use hvmemul_map_linear_addr(), i.e. in both cases
> I'd need to also fiddle with __hvm_copy() (perhaps by making the
> construct below a helper function).
>
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -1324,7 +1324,14 @@ static int hvmemul_write(
> if ( !mapping )
> return linear_write(addr, bytes, p_data, pfec, hvmemul_ctxt);
>
> - memcpy(mapping, p_data, bytes);
> + /* Where possible use single (and hence generally atomic) MOV insns. */
> + switch ( bytes )
> + {
> + case 2: write_u16_atomic(mapping, *(uint16_t *)p_data); break;
> + case 4: write_u32_atomic(mapping, *(uint32_t *)p_data); break;
> + case 8: write_u64_atomic(mapping, *(uint64_t *)p_data); break;
> + default: memcpy(mapping, p_data, bytes); break;
> + }
>
> hvmemul_unmap_linear_addr(mapping, addr, bytes, hvmemul_ctxt);
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Xen-devel] Ping: [PATCH] x86/HVM: use single (atomic) MOV for aligned emulated writes
2019-12-20 16:23 ` [Xen-devel] Ping: " Jan Beulich
@ 2019-12-27 16:06 ` Andrew Cooper
2020-01-13 19:40 ` Jason Andryuk
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Cooper @ 2019-12-27 16:06 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Wei Liu, Roger Pau Monné
On 20/12/2019 16:23, Jan Beulich wrote:
> On 16.09.2019 11:40, Jan Beulich wrote:
>> Using memcpy() may result in multiple individual byte accesses
>> (dependening how memcpy() is implemented and how the resulting insns,
>> e.g. REP MOVSB, get carried out in hardware), which isn't what we
>> want/need for carrying out guest insns as correctly as possible. Fall
>> back to memcpy() only for accesses not 2, 4, or 8 bytes in size.
>>
>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Xen-devel] Ping: [PATCH] x86/HVM: use single (atomic) MOV for aligned emulated writes
2019-12-27 16:06 ` Andrew Cooper
@ 2020-01-13 19:40 ` Jason Andryuk
2020-01-14 9:28 ` [Xen-devel] " Jan Beulich
0 siblings, 1 reply; 6+ messages in thread
From: Jason Andryuk @ 2020-01-13 19:40 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Wei Liu, Jan Beulich, Roger Pau Monné
On Fri, Dec 27, 2019 at 11:09 AM Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
>
> On 20/12/2019 16:23, Jan Beulich wrote:
> > On 16.09.2019 11:40, Jan Beulich wrote:
> >> Using memcpy() may result in multiple individual byte accesses
> >> (dependening how memcpy() is implemented and how the resulting insns,
> >> e.g. REP MOVSB, get carried out in hardware), which isn't what we
> >> want/need for carrying out guest insns as correctly as possible. Fall
> >> back to memcpy() only for accesses not 2, 4, or 8 bytes in size.
> >>
> >> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Should xen/arch/x86/mm/shadow/hvm.c:hvm_emulate_write() be similarly changed?
Thanks,
Jason
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Xen-devel] [PATCH] x86/HVM: use single (atomic) MOV for aligned emulated writes
2020-01-13 19:40 ` Jason Andryuk
@ 2020-01-14 9:28 ` Jan Beulich
2020-01-14 13:51 ` Jason Andryuk
0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2020-01-14 9:28 UTC (permalink / raw)
To: Jason Andryuk; +Cc: Andrew Cooper, Roger Pau Monné, Wei Liu, xen-devel
On 13.01.2020 20:40, Jason Andryuk wrote:
> On Fri, Dec 27, 2019 at 11:09 AM Andrew Cooper
> <andrew.cooper3@citrix.com> wrote:
>>
>> On 20/12/2019 16:23, Jan Beulich wrote:
>>> On 16.09.2019 11:40, Jan Beulich wrote:
>>>> Using memcpy() may result in multiple individual byte accesses
>>>> (dependening how memcpy() is implemented and how the resulting insns,
>>>> e.g. REP MOVSB, get carried out in hardware), which isn't what we
>>>> want/need for carrying out guest insns as correctly as possible. Fall
>>>> back to memcpy() only for accesses not 2, 4, or 8 bytes in size.
>>>>
>>>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> Should xen/arch/x86/mm/shadow/hvm.c:hvm_emulate_write() be similarly changed?
Probably. Care to make a patch?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Xen-devel] [PATCH] x86/HVM: use single (atomic) MOV for aligned emulated writes
2020-01-14 9:28 ` [Xen-devel] " Jan Beulich
@ 2020-01-14 13:51 ` Jason Andryuk
0 siblings, 0 replies; 6+ messages in thread
From: Jason Andryuk @ 2020-01-14 13:51 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Roger Pau Monné, Wei Liu, xen-devel
On Tue, Jan 14, 2020 at 4:28 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 13.01.2020 20:40, Jason Andryuk wrote:
> > On Fri, Dec 27, 2019 at 11:09 AM Andrew Cooper
> > <andrew.cooper3@citrix.com> wrote:
> >>
> >> On 20/12/2019 16:23, Jan Beulich wrote:
> >>> On 16.09.2019 11:40, Jan Beulich wrote:
> >>>> Using memcpy() may result in multiple individual byte accesses
> >>>> (dependening how memcpy() is implemented and how the resulting insns,
> >>>> e.g. REP MOVSB, get carried out in hardware), which isn't what we
> >>>> want/need for carrying out guest insns as correctly as possible. Fall
> >>>> back to memcpy() only for accesses not 2, 4, or 8 bytes in size.
> >>>>
> >>>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >>
> >> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >
> > Should xen/arch/x86/mm/shadow/hvm.c:hvm_emulate_write() be similarly changed?
>
> Probably. Care to make a patch?
Sure :)
-Jason
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-01-14 13:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-16 9:40 [Xen-devel] [PATCH] x86/HVM: use single (atomic) MOV for aligned emulated writes Jan Beulich
2019-12-20 16:23 ` [Xen-devel] Ping: " Jan Beulich
2019-12-27 16:06 ` Andrew Cooper
2020-01-13 19:40 ` Jason Andryuk
2020-01-14 9:28 ` [Xen-devel] " Jan Beulich
2020-01-14 13:51 ` Jason Andryuk
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).