xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86/shim: Simplify compat handling in write_start_info()
@ 2021-04-19 14:45 Andrew Cooper
  2021-04-19 15:55 ` Jan Beulich
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Cooper @ 2021-04-19 14:45 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu

Factor out a compat boolean to remove the lfence overhead from multiple
is_pv_32bit_domain() calls.

For a compat guest, the upper 32 bits of rdx are zero, so there is no need to
have any conditional logic at all when mapping the start info page.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
 xen/arch/x86/pv/shim.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
index d16c0048c0..533c194eef 100644
--- a/xen/arch/x86/pv/shim.c
+++ b/xen/arch/x86/pv/shim.c
@@ -280,12 +280,12 @@ void __init pv_shim_setup_dom(struct domain *d, l4_pgentry_t *l4start,
 static void write_start_info(struct domain *d)
 {
     struct cpu_user_regs *regs = guest_cpu_user_regs();
-    start_info_t *si = map_domain_page(_mfn(is_pv_32bit_domain(d) ? regs->edx
-                                                                  : regs->rdx));
+    bool compat = is_pv_32bit_domain(d);
+    start_info_t *si = map_domain_page(_mfn(regs->rdx));
     uint64_t param;
 
     snprintf(si->magic, sizeof(si->magic), "xen-3.0-x86_%s",
-             is_pv_32bit_domain(d) ? "32p" : "64");
+             compat ? "32p" : "64");
     si->nr_pages = domain_tot_pages(d);
     si->shared_info = virt_to_maddr(d->shared_info);
     si->flags = 0;
@@ -300,7 +300,7 @@ static void write_start_info(struct domain *d)
                                           &si->console.domU.mfn) )
         BUG();
 
-    if ( is_pv_32bit_domain(d) )
+    if ( compat )
         xlat_start_info(si, XLAT_start_info_console_domU);
 
     unmap_domain_page(si);
-- 
2.11.0



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] x86/shim: Simplify compat handling in write_start_info()
  2021-04-19 14:45 [PATCH] x86/shim: Simplify compat handling in write_start_info() Andrew Cooper
@ 2021-04-19 15:55 ` Jan Beulich
  2021-04-19 15:57   ` Andrew Cooper
  0 siblings, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2021-04-19 15:55 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Roger Pau Monné, Wei Liu, Xen-devel

On 19.04.2021 16:45, Andrew Cooper wrote:
> Factor out a compat boolean to remove the lfence overhead from multiple
> is_pv_32bit_domain() calls.
> 
> For a compat guest, the upper 32 bits of rdx are zero, so there is no need to
> have any conditional logic at all when mapping the start info page.

Iirc the contents of the upper halves hold unspecified contents after
a switch from compat to 64-bit mode. Therefore only with this part of
the change dropped ...

> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] x86/shim: Simplify compat handling in write_start_info()
  2021-04-19 15:55 ` Jan Beulich
@ 2021-04-19 15:57   ` Andrew Cooper
  2021-04-19 16:00     ` Jan Beulich
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Cooper @ 2021-04-19 15:57 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Roger Pau Monné, Wei Liu, Xen-devel

On 19/04/2021 16:55, Jan Beulich wrote:
> On 19.04.2021 16:45, Andrew Cooper wrote:
>> Factor out a compat boolean to remove the lfence overhead from multiple
>> is_pv_32bit_domain() calls.
>>
>> For a compat guest, the upper 32 bits of rdx are zero, so there is no need to
>> have any conditional logic at all when mapping the start info page.
> Iirc the contents of the upper halves hold unspecified contents after
> a switch from compat to 64-bit mode. Therefore only with this part of
> the change dropped ...

But we're shim, so will never ever mix compat and non-compat guests.

~Andrew

>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>
> Jan



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] x86/shim: Simplify compat handling in write_start_info()
  2021-04-19 15:57   ` Andrew Cooper
@ 2021-04-19 16:00     ` Jan Beulich
  2021-04-20 17:37       ` Andrew Cooper
  0 siblings, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2021-04-19 16:00 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Roger Pau Monné, Wei Liu, Xen-devel

On 19.04.2021 17:57, Andrew Cooper wrote:
> On 19/04/2021 16:55, Jan Beulich wrote:
>> On 19.04.2021 16:45, Andrew Cooper wrote:
>>> Factor out a compat boolean to remove the lfence overhead from multiple
>>> is_pv_32bit_domain() calls.
>>>
>>> For a compat guest, the upper 32 bits of rdx are zero, so there is no need to
>>> have any conditional logic at all when mapping the start info page.
>> Iirc the contents of the upper halves hold unspecified contents after
>> a switch from compat to 64-bit mode. Therefore only with this part of
>> the change dropped ...
> 
> But we're shim, so will never ever mix compat and non-compat guests.

That's not the point: A compat guest will still cause the CPU to
transition back and forth between 64-bit and compat modes. It is
this transitioning which leaves the upper halves of all GPRs in
undefined state (even if in reality a CPU would likely need to go
through extra hoops to prevent them from being zero if they were
written to in compat mode).

Jan


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] x86/shim: Simplify compat handling in write_start_info()
  2021-04-19 16:00     ` Jan Beulich
@ 2021-04-20 17:37       ` Andrew Cooper
  0 siblings, 0 replies; 5+ messages in thread
From: Andrew Cooper @ 2021-04-20 17:37 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Roger Pau Monné, Wei Liu, Xen-devel

On 19/04/2021 17:00, Jan Beulich wrote:
> On 19.04.2021 17:57, Andrew Cooper wrote:
>> On 19/04/2021 16:55, Jan Beulich wrote:
>>> On 19.04.2021 16:45, Andrew Cooper wrote:
>>>> Factor out a compat boolean to remove the lfence overhead from multiple
>>>> is_pv_32bit_domain() calls.
>>>>
>>>> For a compat guest, the upper 32 bits of rdx are zero, so there is no need to
>>>> have any conditional logic at all when mapping the start info page.
>>> Iirc the contents of the upper halves hold unspecified contents after
>>> a switch from compat to 64-bit mode. Therefore only with this part of
>>> the change dropped ...
>> But we're shim, so will never ever mix compat and non-compat guests.
> That's not the point: A compat guest will still cause the CPU to
> transition back and forth between 64-bit and compat modes. It is
> this transitioning which leaves the upper halves of all GPRs in
> undefined state (even if in reality a CPU would likely need to go
> through extra hoops to prevent them from being zero if they were
> written to in compat mode).

Hmm.  That's awkward.

So real behaviour (I've checked with some contacts) is that upper bits
are preserved until the next write to the register, after which the
upper bits are zeroed.

I wonder whether I'll have any luck formally asking AMD and Intel for a
tweak to this effect in the manuals.

~Andrew



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-04-20 17:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-19 14:45 [PATCH] x86/shim: Simplify compat handling in write_start_info() Andrew Cooper
2021-04-19 15:55 ` Jan Beulich
2021-04-19 15:57   ` Andrew Cooper
2021-04-19 16:00     ` Jan Beulich
2021-04-20 17:37       ` Andrew Cooper

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).