Re: [Xen-devel] [RFC PATCH V2 3/4] hvmload: Add x2apic entry support in the MADT build

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Sep 04, 2017 at 06:59:24PM +0800, Lan Tianyu wrote:
> On 2017年09月01日 17:57, Roger Pau Monné wrote:
> > On Thu, Aug 31, 2017 at 01:01:48AM -0400, Lan Tianyu wrote:
> >> @@ -92,7 +92,12 @@ static struct acpi_20_madt *construct_madt(struct 
> >> acpi_ctxt *ctxt,
> >>      sz  = sizeof(struct acpi_20_madt);
> >>      sz += sizeof(struct acpi_20_madt_intsrcovr) * 16;
> >>      sz += sizeof(struct acpi_20_madt_ioapic);
> >> -    sz += sizeof(struct acpi_20_madt_lapic) * hvminfo->nr_vcpus;
> >> +
> >> +    if (hvminfo->nr_vcpus < 128)
> > 
> > This should be done based on APIC ID.
> 
> There will be a problem how to get max apic id. Should we use the max
> vcpu index to get max APIC id?

IMHO, this should become a loop that iterates over each vCPU getting
it's APIC ID, and add either a lapic or x2apic struct size to sz.

> >> -    info->madt_lapic0_addr = ctxt->mem_ops.v2p(ctxt, lapic);
> >>      for ( i = 0; i < hvminfo->nr_vcpus; i++ )
> >>      {
> >> -        memset(lapic, 0, sizeof(*lapic));
> >> -        lapic->type    = ACPI_PROCESSOR_LOCAL_APIC;
> >> -        lapic->length  = sizeof(*lapic);
> >> -        /* Processor ID must match processor-object IDs in the DSDT. */
> >> -        lapic->acpi_processor_id = i;
> >> -        lapic->apic_id = config->lapic_id(i);
> >> -        lapic->flags = (test_bit(i, hvminfo->vcpu_online)
> >> -                        ? ACPI_LOCAL_APIC_ENABLED : 0);
> >> -        lapic++;
> >> +        unsigned int apic_id = config->lapic_id(i);
> >> +
> >> +        if ( apic_id < 255 ) {
> >> +            struct acpi_20_madt_lapic *lapic = end;
> >> +
> >> +            memset(lapic, 0, sizeof(*lapic));
> >> +            lapic->type    = ACPI_PROCESSOR_LOCAL_APIC;
> >> +            lapic->length  = sizeof(*lapic);
> >> +            /* Processor ID must match processor-object IDs in the DSDT. 
> >> */
> >> +            lapic->acpi_processor_id = i;
> >> +            lapic->apic_id = config->lapic_id(i);
> >> +            lapic->flags = ((i < hvminfo->nr_vcpus) &&
> > 
> > I don't understand why you have added this.
> 
> If apic_id < 255, still use xapic structure and use x2apic structure for
> others. I think this is following your comment on last version.

I'm complaining about the (i < hvminfo->nr_vcpus) check, from the loop
above i is bounded to hvminfo->nr_vpcu, so this check is useless
AFAICT.

Roger.

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