Re: [Xen-devel] [PATCH 1/2] Xen acpi pad implement

["Liu, Jinsong" <jinsong.liu@xxxxxxxxx>]

It's doable to register a stub for xen. However, it's not preferred, say, to 
make xen pad as module, considering the case 'rmmod xen_acpi_pad' then 'insmod 
acpi_pad'? Under such case there is risk of mwait #UD, if we want to remove 
mwait check as 'patch 2/2: Revert-pad-config-check-in-xen_check_mwait.patch' 
did, advantages of which is to enjoy deep Cx.

IMHO to prevent mwait #UD, native pad should never have chance to register 
successfully when running under Xen. So it's better never unregister/disable 
xen pad.

Konrad Rzeszutek Wilk wrote:
>> --- a/drivers/xen/Makefile
>> +++ b/drivers/xen/Makefile
>> @@ -29,6 +29,7 @@ obj-$(CONFIG_XEN_MCE_LOG)          += mcelog.o
>>  obj-$(CONFIG_XEN_PCIDEV_BACKEND)    += xen-pciback/
>>  obj-$(CONFIG_XEN_PRIVCMD)           += xen-privcmd.o
>>  obj-$(CONFIG_XEN_ACPI_PROCESSOR)    += xen-acpi-processor.o
>> +obj-$(CONFIG_XEN_DOM0)                      += xen_acpi_pad.o
> 
> We should have a Kconfig option. In it, please mention what
> version of hypervisor supports this functionality.
> 

Kconfig option for xen pad is not preferred, considering if we disable xen pad 
and then register native pad driver? (of course the precondition is that we do 
not register stub for xen).

>> +#include <linux/kernel.h>
>> +#include <linux/types.h>
>> +#include <acpi/acpi_bus.h>
>> +#include <acpi/acpi_drivers.h>
>> +#include <asm/xen/hypercall.h>
>> +
>> +#if defined(CONFIG_ACPI_PROCESSOR_AGGREGATOR) || \
>> +            defined(CONFIG_ACPI_PROCESSOR_AGGREGATOR_MODULE)
> 
> I don't think you need that.
> 

OK.

>> +
>> +static int __init xen_acpi_pad_init(void)
>> +{
>> +    /* Only DOM0 is responsible for Xen acpi pad */
>> +    if (xen_initial_domain())
>> +            return acpi_bus_register_driver(&xen_acpi_pad_driver); +
>> +    return -ENODEV;
>> +}
>> +subsys_initcall(xen_acpi_pad_init);
> 
> 
> No way of making this a module? It would be nice - perhaps have a stub
> function that registers the bus but does not do anything until this
> proper module is loaded?
> 

It's doable but not preferred, reason as above.

>> +
>> +#endif
>> diff --git a/include/xen/interface/platform.h
>> b/include/xen/interface/platform.h index 4755b5f..0f44376 100644 ---
>> a/include/xen/interface/platform.h +++
>> b/include/xen/interface/platform.h @@ -324,6 +324,22 @@ struct
>>  xenpf_cpu_ol { };
>>  DEFINE_GUEST_HANDLE_STRUCT(xenpf_cpu_ol);
>> 
>> +/*
>> + * CMD 58 and 59 are reserved for cpu hotadd and memory hotadd,
>> + * which already occupied at Xen hypervisor side.
>            ^- are
> 

Sure.

Thanks,
Jinsong


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel