Re: [RFC PATCH] xen, apic: Setup our own APIC driver and validator for APIC IDs.

[David Vrabel <david.vrabel@citrix.com>]

On 21/01/15 21:56, Konrad Rzeszutek Wilk wrote:
> +static struct apic xen_apic = {
> +	.name = "Xen",
> +	.probe = probe_xen,
> +	/* The rest is copied from the default. */

Explicitly initialize all required members here.  memcpy'ing from the
default makes it far too unclear which ops this apic driver actually
provides.

> +};
> +
> +/*
> + * This is needed as in enlighten.c we mask the x2APIC bit because we
> + * do not want PV guests to use anything but most of the default apic routines.
> + *
> + * However the default ->apic_id_valid enforces that the APIC ID MUST
> + * be below 0xFF which is not the case for x2APIC - so we need a way
> + * to allow that to function properly.
> + */
> +static bool __init xen_check_x2apic(void)
> +{
> +#ifdef CONFIG_X2APIC
> +	unsigned int ax, bx, cx, dx;
> +
> +	ax = 1;
> +	cx = 0; /* Don't care about dx, and bx */
> +	native_cpuid(&ax, &bx, &cx, &dx);
> +	if (cx & (1 << (X86_FEATURE_X2APIC % 32)))
> +		return true;
> +#endif
> +	return false;
> +}

Not needed (see below).

>  void __init xen_init_apic(void)
>  {
>  	x86_io_apic_ops.read = xen_io_apic_read;
> +
> +	memcpy(&xen_apic, apic, sizeof(struct apic));
> +	xen_apic.probe = probe_xen;
> +	xen_apic.name = "Xen";
> +
> +	xen_apic.read = xen_apic_read;
> +	xen_apic.write = xen_apic_write;
> +	xen_apic.icr_read = xen_apic_icr_read;
> +	xen_apic.icr_write = xen_apic_icr_write;
> +	xen_apic.wait_icr_idle = xen_apic_wait_icr_idle;
> +	xen_apic.safe_wait_icr_idle = xen_safe_apic_wait_icr_idle;
> +	xen_apic.set_apic_id = xen_set_apic_id;
> +	xen_apic.get_apic_id = xen_get_apic_id;
> +
> +	xen_apic.send_IPI_allbutself = xen_send_IPI_allbutself;
> +	xen_apic.send_IPI_mask_allbutself = xen_send_IPI_mask_allbutself;
> +	xen_apic.send_IPI_mask = xen_send_IPI_mask;
> +	xen_apic.send_IPI_all = xen_send_IPI_all;
> +	xen_apic.send_IPI_self = xen_send_IPI_self;
> +
> +	if (xen_check_x2apic())
> +		xen_apic.apic_id_valid = xen_id_always_valid;

Just always use xen_id_always_valid regardless of whether the machine
has x2apic or not.  It is possible to have more VCPUs that PCPUs.

David