protmode_load_seg() would better adhere to that "feature" of clearing
base (and limit) during NULL selector loads.

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

--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1970,6 +1970,7 @@ amd_like(const struct x86_emulate_ctxt *
 #define vcpu_has_tbm()         (ctxt->cpuid->extd.tbm)
 #define vcpu_has_clzero()      (ctxt->cpuid->extd.clzero)
 #define vcpu_has_wbnoinvd()    (ctxt->cpuid->extd.wbnoinvd)
+#define vcpu_has_nscb()        (ctxt->cpuid->extd.nscb)
 
 #define vcpu_has_bmi1()        (ctxt->cpuid->feat.bmi1)
 #define vcpu_has_hle()         (ctxt->cpuid->feat.hle)
@@ -2102,7 +2103,7 @@ protmode_load_seg(
         case x86_seg_tr:
             goto raise_exn;
         }
-        if ( !_amd_like(cp) || !ops->read_segment ||
+        if ( !_amd_like(cp) || vcpu_has_nscb() || !ops->read_segment ||
              ops->read_segment(seg, sreg, ctxt) != X86EMUL_OKAY )
             memset(sreg, 0, sizeof(*sreg));
         else

On 15/09/2022 08:22, Jan Beulich wrote:
> protmode_load_seg() would better adhere to that "feature" of clearing
> base (and limit) during NULL selector loads.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

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

On 10.10.2022 18:44, Andrew Cooper wrote:
> On 15/09/2022 08:22, Jan Beulich wrote:
>> protmode_load_seg() would better adhere to that "feature" of clearing
>> base (and limit) during NULL selector loads.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Thanks.

Henry - this was submitted before the code freeze, so you weren't Cc-ed.
May I ask to consider giving this a release ack?

Thanks, Jan

Hi Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Subject: [4.17?] Re: [PATCH] x86emul: respect NSCB
> 
> On 10.10.2022 18:44, Andrew Cooper wrote:
> > On 15/09/2022 08:22, Jan Beulich wrote:
> >> protmode_load_seg() would better adhere to that "feature" of clearing
> >> base (and limit) during NULL selector loads.
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >
> > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Thanks.
> 
> Henry - this was submitted before the code freeze, so you weren't Cc-ed.
> May I ask to consider giving this a release ack?

Since this patch is simple and to my best knowledge this patch is trying to
improve the code so:

Release-acked-by: Henry Wang <Henry.Wang@arm.com>

(If it will not cause too much time of digging, would you mind adding a
"Fixes:" tag pointing to the original commit that missing this
` vcpu_has_nscb()` check when you do the committing? I think this would
help to identify this patch as a bugfix so it is more reasonable to commit
this patch in current phase. But if too much trouble or you think this is
not really a fix then just ignore my comment...)

Kind regards,
Henry

> 
> Thanks, Jan

On 11/10/2022 11:44, Henry Wang wrote:
> Hi Jan,
>
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Subject: [4.17?] Re: [PATCH] x86emul: respect NSCB
>>
>> On 10.10.2022 18:44, Andrew Cooper wrote:
>>> On 15/09/2022 08:22, Jan Beulich wrote:
>>>> protmode_load_seg() would better adhere to that "feature" of clearing
>>>> base (and limit) during NULL selector loads.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Thanks.
>>
>> Henry - this was submitted before the code freeze, so you weren't Cc-ed.
>> May I ask to consider giving this a release ack?
> Since this patch is simple and to my best knowledge this patch is trying to
> improve the code so:
>
> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
>
> (If it will not cause too much time of digging, would you mind adding a
> "Fixes:" tag pointing to the original commit that missing this
> ` vcpu_has_nscb()` check when you do the committing? I think this would
> help to identify this patch as a bugfix so it is more reasonable to commit
> this patch in current phase. But if too much trouble or you think this is
> not really a fix then just ignore my comment...)

There isn't really an appropriate Fixes tag.

This CPUID bit is one I managed to get AMD to retroactively add to fix
an enumeration problem they had no anticipated when making a change in Zen2.

i.e. the CPUID bit did not exist at the point at which the code,
modified in this patch, was written.

~Andrew

Hi Andrew,

> -----Original Message-----
> From: Andrew Cooper <Andrew.Cooper3@citrix.com>
> > (If it will not cause too much time of digging, would you mind adding a
> > "Fixes:" tag pointing to the original commit that missing this
> > ` vcpu_has_nscb()` check when you do the committing? I think this would
> > help to identify this patch as a bugfix so it is more reasonable to commit
> > this patch in current phase. But if too much trouble or you think this is
> > not really a fix then just ignore my comment...)
> 
> There isn't really an appropriate Fixes tag.
> 
> This CPUID bit is one I managed to get AMD to retroactively add to fix
> an enumeration problem they had no anticipated when making a change in
> Zen2.
> 
> i.e. the CPUID bit did not exist at the point at which the code,
> modified in this patch, was written.

Oh, thanks for the clarification! Then please just ignore my comments, sorry
for the noise.

Kind regards,
Henry

> 
> ~Andrew