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

[Liu, Jinsong]

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/