Re: [Xen-devel] [PATCH 2/5] xen: make use of new pae enum in hypervisor

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.09.15 at 13:54, <JGross@xxxxxxxx> wrote:
> --- a/xen/arch/x86/domain_build.c
> +++ b/xen/arch/x86/domain_build.c
> @@ -953,8 +953,8 @@ int __init construct_dom0(
>      compat32   = 0;
>      machine = elf_uval(&elf, elf.ehdr, e_machine);
>      printk(" Xen  kernel: 64-bit, lsb, compat32\n");
> -    if (elf_32bit(&elf) && parms.pae == PAEKERN_bimodal)
> -        parms.pae = PAEKERN_extended_cr3;
> +    if (elf_32bit(&elf) && parms.pae == XEN_PAE_BIMODAL)
> +        parms.pae = XEN_PAE_EXTCR3;

I think this and the first patch can't validly be split: You're altering
the values stored in parms.pae in the first one, but taking this code
together with ...

> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -475,10 +475,6 @@ struct domain_setup_info
>      unsigned long v_kernstart;
>      unsigned long v_kernend;
>      unsigned long v_kernentry;
> -#define PAEKERN_no           0
> -#define PAEKERN_yes          1
> -#define PAEKERN_extended_cr3 2
> -#define PAEKERN_bimodal      3
>      unsigned int  pae_kernel;

... this, the PAEKERN_* values were already meant to be in sync
with the literal numbers used in libelf code.

Furthermore there's no single reference to a pae_kernel field
throughout the xen/ subtree, which suggests the field was already
dead. Even more - it looks like the whole structure is unused; the
only references are apparently dead function declarations in
xen/elf.h. So the two patches should be folded, and the resulting
one extended.

Jan


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