Re: [Xen-devel] [PATCH v2 5/7] x86: relocate pvh_info

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Jan 19, 2018 at 04:29:31PM +0000, Roger Pau Monné wrote:
> On Fri, Jan 19, 2018 at 03:34:56PM +0000, Wei Liu wrote:
> > diff --git a/xen/arch/x86/boot/build32.mk b/xen/arch/x86/boot/build32.mk
> > index 48c7407c00..028ac19b96 100644
> > --- a/xen/arch/x86/boot/build32.mk
> > +++ b/xen/arch/x86/boot/build32.mk
> > @@ -36,5 +36,8 @@ CFLAGS := $(filter-out -flto,$(CFLAGS))
> >  cmdline.o: cmdline.c $(CMDLINE_DEPS)
> >  
> >  reloc.o: reloc.c $(RELOC_DEPS)
> > +ifeq ($(CONFIG_PVH_GUEST),y)
> > +reloc.o: CFLAGS += -DCONFIG_PVH_GUEST
> > +endif
> 
> I would maybe do this above, where the rest of the CFLAGS are set.
> Certainly setting -DCONFIG_PVH_GUEST shouldn't cause issues elsewhere.
> 
> CFLAGS-$(CONFIG_PVH_GUEST) += -DCONFIG_PVH_GUEST
> CFLAGS += $(CFLAGS-y)
> 
> >  .PRECIOUS: %.bin %.lnk
> > diff --git a/xen/arch/x86/boot/defs.h b/xen/arch/x86/boot/defs.h
> > index 6abdc15446..05921a64a3 100644
> > --- a/xen/arch/x86/boot/defs.h
> > +++ b/xen/arch/x86/boot/defs.h
> > @@ -51,6 +51,9 @@ typedef unsigned short u16;
> >  typedef unsigned int u32;
> >  typedef unsigned long long u64;
> >  typedef unsigned int size_t;
> > +typedef u8 uint8_t;
> > +typedef u32 uint32_t;
> > +typedef u64 uint64_t;
> 
> This this seems to be always expanding, maybe better to simply replace
> the stdbool.h include above with types.h?
> 

I'm two minded here. My impression is that this wants to be minimal and
standalone. The content in types.h is a lot more than we need here.

I will let Jan and Andrew decide.

> >  #define U16_MAX            ((u16)(~0U))
> >  #define UINT_MAX   (~0U)
> > diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
> > index 0f652cea11..614e53081e 100644
> > --- a/xen/arch/x86/boot/head.S
> > +++ b/xen/arch/x86/boot/head.S
> > @@ -414,6 +414,7 @@ __pvh_start:
> >  
> >          /* Set trampoline_phys to use mfn 1 to avoid having a mapping at 
> > VA 0 */
> >          movw    $0x1000, sym_esi(trampoline_phys)
> > +        movl    $0x336ec578, %eax /* mov $XEN_HVM_START_MAGIC_VALUE, %eax 
> > */
> 
> Hm, if XEN_HVM_START_MAGIC_VALUE cannot be used I would rather prefer
> to use (%ebx).

The same reason I didn't include types.h + hvm_start_info.h here.

We can include both to make $XEN_HVM_START_MAGIC_VALUE work. But I think
using (%ebx) is better in here.

Wei.

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