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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.10.18 at 16:41, <roger.pau@xxxxxxxxxx> wrote:
> --- 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 amd_acpi_c1e_quirk;

__read_mostly

> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1273,6 +1273,24 @@ void svm_host_osvw_init()
>      spin_unlock(&osvw_lock);
>  }
>  
> +static int acpi_c1e_quirk(int dir, unsigned int port, unsigned int bytes,
> +                          uint32_t *val)
> +{
> +    ASSERT(bytes == 1 && port == acpi_smi_cmd);
> +
> +    if ( dir == IOREQ_READ )
> +    {
> +        *val = inb(port);
> +        return X86EMUL_OKAY;
> +    }
> +
> +    outb(*val, port);
> +    if ( *val == acpi_enable_value )
> +       on_each_cpu(amd_disable_c1e, NULL, 1);
> +
> +    return X86EMUL_OKAY;
> +}

I think you could do with even less logic duplication if instead you
called check_disable_c1e() (then given an amd_ prefix of course)
here. I'd then further consider to do away with
pv_post_outb_hook in a follow-up patch, getting the PV side
closer to the model used here.

Not being the maintainer of this code, I'd nevertheless like to also
suggest to use if/else here, making the function get away with
just a single return point.

Jan



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