| Message ID | 5762903B02000078000F597E@prv-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/16/2016 05:40 AM, Jan Beulich wrote: > The IO-APIC address has variable bits determined by the PCI-to-ISA > bridge, and the IO-APIC version should be read from the IO-APIC. (Note > that there's still implicit rather than explicit agreement on the > IO-APIC base address between qemu and the hypervisor.) It probably doesn't matter now for PVHv2 since we are not going to initially emulate IOAPIC but when/if we do, how do we query for base address and version? Should we just rely on the same implicit agreement? -boris
>>> On 16.06.16 at 18:49, <boris.ostrovsky@oracle.com> wrote: > On 06/16/2016 05:40 AM, Jan Beulich wrote: >> The IO-APIC address has variable bits determined by the PCI-to-ISA >> bridge, and the IO-APIC version should be read from the IO-APIC. (Note >> that there's still implicit rather than explicit agreement on the >> IO-APIC base address between qemu and the hypervisor.) > > It probably doesn't matter now for PVHv2 since we are not going to > initially emulate IOAPIC but when/if we do, how do we query for base > address and version? Should we just rely on the same implicit agreement? If this refers to the sentence in parentheses, then this was really meant to indicate a work item. I.e. qemu should negotiate with Xen. For PVHv2 this would then probably mean for hvmloader to query Xen (via new hypercall). Jan
On 16/06/16 10:40, Jan Beulich wrote: > The IO-APIC address has variable bits determined by the PCI-to-ISA > bridge, and the IO-APIC version should be read from the IO-APIC. (Note > that there's still implicit rather than explicit agreement on the > IO-APIC base address between qemu and the hypervisor.) > > Signed-off-by: Jan Beulich <jbeulich@suse.com> The status quo is not great, and I can see why you want to improve it. However, I think that this is not the way to do that. It ties HVMLoader to the PIIX4 board in Qemu, and will break attempts to use Q35 or something else. (In Q35, the IO-APIC decode address comes from Chipset Configuration Register, rather than ISA device config space). ~Andrew
>>> On 17.06.16 at 12:05, <andrew.cooper3@citrix.com> wrote: > On 16/06/16 10:40, Jan Beulich wrote: >> The IO-APIC address has variable bits determined by the PCI-to-ISA >> bridge, and the IO-APIC version should be read from the IO-APIC. (Note >> that there's still implicit rather than explicit agreement on the >> IO-APIC base address between qemu and the hypervisor.) >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > The status quo is not great, and I can see why you want to improve it. > > However, I think that this is not the way to do that. It ties HVMLoader > to the PIIX4 board in Qemu, and will break attempts to use Q35 or > something else. (In Q35, the IO-APIC decode address comes from Chipset > Configuration Register, rather than ISA device config space). hvmloader is already tied to PIIX4, and enabling Q35 will, among other things, require hvmloader to become aware of that chipset. Jan
On 06/17/2016 02:00 AM, Jan Beulich wrote: >>>> On 16.06.16 at 18:49, <boris.ostrovsky@oracle.com> wrote: >> On 06/16/2016 05:40 AM, Jan Beulich wrote: >>> The IO-APIC address has variable bits determined by the PCI-to-ISA >>> bridge, and the IO-APIC version should be read from the IO-APIC. (Note >>> that there's still implicit rather than explicit agreement on the >>> IO-APIC base address between qemu and the hypervisor.) >> It probably doesn't matter now for PVHv2 since we are not going to >> initially emulate IOAPIC but when/if we do, how do we query for base >> address and version? Should we just rely on the same implicit agreement? > If this refers to the sentence in parentheses, then this was really > meant to indicate a work item. I.e. qemu should negotiate with > Xen. For PVHv2 this would then probably mean for hvmloader to > query Xen (via new hypercall). We are not using hvmloader for PVH so I am not sure how that would work. You mean the toolstack? -boris
>>> On 17.06.16 at 13:51, <boris.ostrovsky@oracle.com> wrote: > On 06/17/2016 02:00 AM, Jan Beulich wrote: >>>>> On 16.06.16 at 18:49, <boris.ostrovsky@oracle.com> wrote: >>> On 06/16/2016 05:40 AM, Jan Beulich wrote: >>>> The IO-APIC address has variable bits determined by the PCI-to-ISA >>>> bridge, and the IO-APIC version should be read from the IO-APIC. (Note >>>> that there's still implicit rather than explicit agreement on the >>>> IO-APIC base address between qemu and the hypervisor.) >>> It probably doesn't matter now for PVHv2 since we are not going to >>> initially emulate IOAPIC but when/if we do, how do we query for base >>> address and version? Should we just rely on the same implicit agreement? >> If this refers to the sentence in parentheses, then this was really >> meant to indicate a work item. I.e. qemu should negotiate with >> Xen. For PVHv2 this would then probably mean for hvmloader to >> query Xen (via new hypercall). > > We are not using hvmloader for PVH so I am not sure how that would work. > You mean the toolstack? Oh, right. Whoever puts the ACPI tables in place. Jan
--- a/tools/firmware/hvmloader/acpi/build.c +++ b/tools/firmware/hvmloader/acpi/build.c @@ -138,7 +138,7 @@ static struct acpi_20_madt *construct_ma io_apic->type = ACPI_IO_APIC; io_apic->length = sizeof(*io_apic); io_apic->ioapic_id = IOAPIC_ID; - io_apic->ioapic_addr = IOAPIC_BASE_ADDRESS; + io_apic->ioapic_addr = ioapic_base_address; lapic = (struct acpi_20_madt_lapic *)(io_apic + 1); info->nr_cpus = hvm_info->nr_vcpus; --- a/tools/firmware/hvmloader/config.h +++ b/tools/firmware/hvmloader/config.h @@ -42,9 +42,10 @@ extern struct bios_config ovmf_config; #define PAGE_SHIFT 12 #define PAGE_SIZE (1ul << PAGE_SHIFT) -#define IOAPIC_BASE_ADDRESS 0xfec00000 +extern uint32_t ioapic_base_address; +extern uint8_t ioapic_version; + #define IOAPIC_ID 0x01 -#define IOAPIC_VERSION 0x11 #define LAPIC_BASE_ADDRESS 0xfee00000 #define LAPIC_ID(vcpu_id) ((vcpu_id) * 2) --- a/tools/firmware/hvmloader/hvmloader.c +++ b/tools/firmware/hvmloader/hvmloader.c @@ -108,6 +108,9 @@ asm ( unsigned long scratch_start = SCRATCH_PHYSICAL_ADDRESS; +uint32_t ioapic_base_address = 0xfec00000; +uint8_t ioapic_version; + static void init_hypercalls(void) { uint32_t eax, ebx, ecx, edx; @@ -185,6 +188,9 @@ static void init_vm86_tss(void) static void apic_setup(void) { + ioapic_base_address |= (pci_readb(PCI_ISA_DEVFN, 0x80) & 0x3f) << 10; + ioapic_version = ioapic_read(0x01) & 0xff; + /* Set the IOAPIC ID to the static value used in the MP/ACPI tables. */ ioapic_write(0x00, IOAPIC_ID); --- a/tools/firmware/hvmloader/mp_tables.c +++ b/tools/firmware/hvmloader/mp_tables.c @@ -227,9 +227,9 @@ static void fill_mp_ioapic_entry(struct { mpie->type = ENTRY_TYPE_IOAPIC; mpie->ioapic_id = IOAPIC_ID; - mpie->ioapic_version = IOAPIC_VERSION; + mpie->ioapic_version = ioapic_version; mpie->ioapic_flags = 1; /* enabled */ - mpie->ioapic_addr = IOAPIC_BASE_ADDRESS; + mpie->ioapic_addr = ioapic_base_address; } --- a/tools/firmware/hvmloader/util.c +++ b/tools/firmware/hvmloader/util.c @@ -490,14 +490,14 @@ void *scratch_alloc(uint32_t size, uint3 uint32_t ioapic_read(uint32_t reg) { - *(volatile uint32_t *)(IOAPIC_BASE_ADDRESS + 0x00) = reg; - return *(volatile uint32_t *)(IOAPIC_BASE_ADDRESS + 0x10); + *(volatile uint32_t *)(ioapic_base_address + 0x00) = reg; + return *(volatile uint32_t *)(ioapic_base_address + 0x10); } void ioapic_write(uint32_t reg, uint32_t val) { - *(volatile uint32_t *)(IOAPIC_BASE_ADDRESS + 0x00) = reg; - *(volatile uint32_t *)(IOAPIC_BASE_ADDRESS + 0x10) = val; + *(volatile uint32_t *)(ioapic_base_address + 0x00) = reg; + *(volatile uint32_t *)(ioapic_base_address + 0x10) = val; } uint32_t lapic_read(uint32_t reg)