Re: [Xen-devel] [PATCH v4 03/16] libxl/arm: Generate static ACPI DSDT table

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 31 Aug 2016, Julien Grall wrote:
> Hi Shannon,
> 
> On 31/08/16 07:37, Shannon Zhao wrote:
> > On 2016/8/30 1:46, Julien Grall wrote:
> > > On 16/08/2016 06:25, Shannon Zhao wrote:
> > > > From: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
> > > > 
> > > > It uses static DSDT table like the way x86 uses. Currently the DSDT
> > > > table only contains processor device objects and it generates the
> > > > maximal objects which so far is 128.
> > > > 
> > > > Also only check iasl for aarch64 in configure since ACPI on ARM32 is not
> > > > supported.
> > > > 
> > > > Signed-off-by: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
> > > > ---
> > > >  tools/configure               |  2 +-
> > > 
> > > The file tools/configure should not be modified manually. Instead you
> > > have to modify tools/configure.ac.
> > > 
> > > You can regenerate tools/configure, you can call ./autegen.sh. However,
> > > I would recommend you to not include the changes of configure and ask
> > > the committer to regenerate. This is because we use always use the same
> > > version of autotools to do generation in order to avoid spurious change.
> > > 
> > Ok, will fix.
> > 
> > > > diff --git a/xen/include/public/arch-arm.h
> > > > b/xen/include/public/arch-arm.h
> > > > index 0afd654..008a2a0 100644
> > > > --- a/xen/include/public/arch-arm.h
> > > > +++ b/xen/include/public/arch-arm.h
> > > > @@ -435,6 +435,9 @@ typedef uint64_t xen_callback_t;
> > > >  #define GUEST_RAM_BANK_BASES   { GUEST_RAM0_BASE, GUEST_RAM1_BASE }
> > > >  #define GUEST_RAM_BANK_SIZES   { GUEST_RAM0_SIZE, GUEST_RAM1_SIZE }
> > > > 
> > > > +/* Current supported guest VCPUs */
> > > > +#define GUEST_MAX_VCPUS 128
> > > 
> > > The number of vCPUS per guest supported depends whether Xen has been
> > > built for ARM32 or ARM64.
> > > 
> > > Also, because now we have two different place to define the number of
> > > vCPUS (here and include/asm-arm/config.h) it might be possible to have
> > > them differ by mistake.
> > > 
> > > I am not sure how to avoid the 2 definitions, so I would add a
> > > BUILD_BUG_ON in Xen to make sure that MAX_VIRT_CPUS is always <= to
> > > GUEST_MAX_VCPUS.
> > > 
> > It has the below check. So could we just define GUEST_MAX_VCPUS as
> > (GUEST_GICV3_GICR0_SIZE / GUEST_GICV3_RDIST_STRIDE)?
> 
> I much prefer hardcoding the value. It will be easier to catch any issue if we
> decide to use multiple re-distributor regions.
> 
> Stefano, do you have any opinions?

I agree with you. It is going to be much easier to catch any mistakes
with the hardcoded value.


> > 
> > BUILD_BUG_ON((GUEST_GICV3_GICR0_SIZE / GUEST_GICV3_RDIST_STRIDE) <
> > MAX_VIRT_CPUS);
> > 
> > Thanks,

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