Re: [Xen-devel] [PATCH v4] amd/pvh: enable ACPI C1E disable quirk on PVH Dom0

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Nov 08, 2018 at 03:52:21PM +0100, Roger Pau Monné wrote:
> On Thu, Nov 08, 2018 at 02:42:27PM +0000, Wei Liu wrote:
> > On Thu, Nov 08, 2018 at 03:23:58PM +0100, Roger Pau Monne wrote:
> > > PV Dom0 has a quirk for some AMD processors, where enabling ACPI can
> > > also enable C1E mode. Apply the same workaround as done on PV for a
> > > PVH Dom0, which consist on trapping accesses to the SMI command IO
> > > port and disabling C1E if ACPI is enabled.
> > > 
> > > Reported-by: Jan Beulich <jbeulich@xxxxxxxx>
> > > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > 
> > Reviewed-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> > 
> > 
> > > ---
> > > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > Cc: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> > > Cc: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> > > Cc: Brian Woods <brian.woods@xxxxxxx>
> > > ---
> > > Changes since v3:
> > >  - Mark amd_acpi_c1e_quirk as read mostly.
> > >  - Use check_disable_c1e instead of disable_c1e.
> > >  - Use a single return path in acpi_c1e_quirk.
> > > 
> > > Changes since v2:
> > >  - Only register the IO port handler for the hardware domain.
> > > ---
> > >  xen/arch/x86/cpu/amd.c     | 11 ++++++++---
> > >  xen/arch/x86/dom0_build.c  |  5 +++++
> > >  xen/arch/x86/hvm/svm/svm.c | 19 +++++++++++++++++++
> > >  xen/include/asm-x86/amd.h  |  3 +++
> > >  4 files changed, 35 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
> > > index c394c1c2ec..8895c25682 100644
> > > --- a/xen/arch/x86/cpu/amd.c
> > > +++ b/xen/arch/x86/cpu/amd.c
> > > @@ -44,6 +44,9 @@ integer_param("cpuid_mask_thermal_ecx", 
> > > opt_cpuid_mask_thermal_ecx);
> > >  s8 __read_mostly opt_allow_unsafe;
> > >  boolean_param("allow_unsafe", opt_allow_unsafe);
> > >  
> > > +/* Signal whether the ACPI C1E quirk is required. */
> > > +bool __read_mostly amd_acpi_c1e_quirk;
> > > +
> > >  static inline int rdmsr_amd_safe(unsigned int msr, unsigned int *lo,
> > >                            unsigned int *hi)
> > >  {
> > > @@ -443,7 +446,7 @@ static void disable_c1e(void *unused)
> > >                  smp_processor_id(), msr_content);
> > >  }
> > >  
> > > -static void check_disable_c1e(unsigned int port, u8 value)
> > > +void amd_check_disable_c1e(unsigned int port, u8 value)
> > 
> > One minor cosmetic issue: it would have been nice to change u8 to
> > uint8_t here and below. I think this can be fixed while committing.
> 
> I thought about that, but the whole amd.c file has a different coding
> style from the Xen hypervisor style, so I decided to leave it as-is
> because I didn't know if that was due to the file being kept in sync
> with some external source.

Fair enough. I'm not too fussed about whether we change it or not then.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel