Message ID | b0103a30-22b7-c010-3e8b-6eab1c25ee47@suse.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | x86emul: respect NSCB | expand |
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
--- 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
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>