Re: Design doc of adding ACPI support for arm64 on Xen - version 5

[Ian Campbell <ian.campbell@citrix.com>]

On Wed, 2015-09-02 at 21:59 +0800, Shannon Zhao wrote:
> Hi Ian,
> 
> On 2015/9/2 20:54, Ian Campbell wrote:
> > On Mon, 2015-08-31 at 03:40 -0600, Jan Beulich wrote:
> > > > d) Copy MADT table
> > > > > > > > It needs to change MADT table to restrict the number of 
> > > > > > > > vCPUs.
> > > > > > > > We choose
> > > > > > > > to copy the first dom0_max_vcpus GICC entries of MADT to 
> > > > > > > > new
> > > > > > > > created
> > > > > > > > MADT table when numa is not supported currently.
> > > > > > > 
> > > > > > > Copy means you imply to have an original?
> > > > > > 
> > > > > > So I'll change it to "create".
> > > > > > 
> > > > > > > What if dom0_max_vcpus
> > > > > > > is larger than the physical CPU count?
> > > > > > 
> > > > > > I think it only needs to care the cpu_interface_number, uid and
> > > > > > mpidr
> > > > > > field of GICC entry and other fields could be same with the 
> > > > > > host
> > > > > > GICC
> > > > > > entry. It could get the mpidr from the vCPU index.
> > > > > 
> > > > > You again suggest to use data from host entries, i.e. you leave
> > > > > incompletely addressed the original question: "What source of
> > > > > information do you intend to use when the Dom0's vCPU count is
> > > > > higher than the host's pCPU count?"
> > > > > 
> > > > 
> > > > There are only the cpu_interface_number, uid and mpidr which need 
> > > > to
> > > > change. Other fields could copy from any one of the GICC entries 
> > > > from
> > > > host MADT table.
> > > 
> > > Hmm, okay, if _any one_ is indeed fine, then okay. But then please
> > > change to wording in your document to make this explicit (and to also
> > > make explicit that you consider this an okay thing to do in the first
> > > place, just to catch others' attention to double check it really is).
> > 
> > In the DT world we completely ignore the host /cpus/* nodes when 
> > creating
> > dom0's /cpus/ and instead create our own directly. It would seem 
> > logical to
> > me to do the same for the MADR in ACPI world as well.
> > 
> > Above the cpu_interface_number, uid and mpidr are called out as things 
> > we
> > we know how to create/determine ourselves.
> > 
> > For the other fields you propose copying from a host entry in the MADT, 
> > but
> > it would be better where possible to create our own. Looking at ACPI 
> > 6.0 I
> > see for the MADT table and the various ARM related structures I'm not
> > immediately seeing any fields which Xen shouldn't be able to decide for
> > itself.
> > 
> > When creating the node we may of course be relying on things (such as d
> > ->arch.vgic or maybe in some small cases gic_hw_ops) which are 
> > initialised
> > (elsewhere) by values from the host, but form the PoV of creating the 
> > MADT
> > this is abstracted away.
> > 
> > So to give some examples:
> > 
> >        * The GIC redistributor addresses come from d
> >          ->arch.vgic.rdist_regions[i].
> >        * the "Parking Protocol Version" ought to be set to reflect 
> > Xen's
> >          support for PSCI, not whatever the host says.
> >        * "Processor Power Efficiency Class" ought to be some number 
> > made up
> >          by Xen for all processors (probably 0), since the power 
> > efficiency
> >          of a VCPU has nothing to do with any particular PCPU and we 
> > assume
> >          (today) that all VCPUs are equivalent.
> >        * The GICD version field comes from d->arch.vgic.version
> > 
> > What, if any, fields do you now think should be copied directly from 
> > the
> > host MADT?
> > 
> Of course we could create the entries according to the information of 
> d->arch.vgic. But the contents of d->arch.vgic are also get from the 
> host MADT table. It really has a big difference between the two ways?

Yes, the difference is in the use of the correct abstractions. Going direct
to the host MADT in the code which builds the dom0 MADT goes around the
abstraction.

> In addition, I don't object to the way you suggest. Both are fine to me.
> 
> Thanks,