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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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.

Thanks, Roger.

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