x86: synchronize PCI config space access decoding Both PV and HVM logic have similar but not similar enough code here. Synchronize the two so that - in the HVM case we don't unconditionally try to access extended config space - in the PV case we pass a correct range to the XSM hook - in the PV case we don't needlessly deny access when the operation isn't really on PCI config space All this along with sharing the macros HVM already had here. Signed-off-by: Jan Beulich --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -2383,11 +2383,6 @@ void hvm_vcpu_down(struct vcpu *v) static struct hvm_ioreq_server *hvm_select_ioreq_server(struct domain *d, ioreq_t *p) { -#define CF8_BDF(cf8) (((cf8) & 0x00ffff00) >> 8) -#define CF8_ADDR_LO(cf8) ((cf8) & 0x000000fc) -#define CF8_ADDR_HI(cf8) (((cf8) & 0x0f000000) >> 16) -#define CF8_ENABLED(cf8) (!!((cf8) & 0x80000000)) - struct hvm_ioreq_server *s; uint32_t cf8; uint8_t type; @@ -2416,9 +2411,19 @@ static struct hvm_ioreq_server *hvm_sele type = IOREQ_TYPE_PCI_CONFIG; addr = ((uint64_t)sbdf << 32) | - CF8_ADDR_HI(cf8) | CF8_ADDR_LO(cf8) | (p->addr & 3); + /* AMD extended configuration space access? */ + if ( CF8_ADDR_HI(cf8) && + boot_cpu_data.x86_vendor == X86_VENDOR_AMD && + boot_cpu_data.x86 >= 0x10 && boot_cpu_data.x86 <= 0x17 ) + { + uint64_t msr_val; + + if ( !rdmsr_safe(MSR_AMD64_NB_CFG, msr_val) && + (msr_val & (1ULL << AMD64_NB_CFG_CF8_EXT_ENABLE_BIT)) ) + addr |= CF8_ADDR_HI(cf8); + } } else { @@ -2472,11 +2477,6 @@ static struct hvm_ioreq_server *hvm_sele } return d->arch.hvm_domain.default_ioreq_server; - -#undef CF8_ADDR_ENABLED -#undef CF8_ADDR_HI -#undef CF8_ADDR_LO -#undef CF8_BDF } int hvm_buffered_io_send(ioreq_t *p) --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -1770,14 +1770,19 @@ static int admin_io_okay( return ioports_access_permitted(v->domain, port, port + bytes - 1); } -static int pci_cfg_ok(struct domain *d, int write, int size) +static bool_t pci_cfg_ok(struct domain *d, bool_t write, + uint16_t start, uint16_t size) { uint32_t machine_bdf; - uint16_t start, end; - if (!is_hardware_domain(d)) + uint16_t end; + + if ( !is_hardware_domain(d) ) return 0; - machine_bdf = (d->arch.pci_cf8 >> 8) & 0xFFFF; + if ( !CF8_ENABLED(d->arch.pci_cf8) ) + return 1; + + machine_bdf = CF8_BDF(d->arch.pci_cf8); if ( write ) { const unsigned long *ro_map = pci_get_ro_map(0); @@ -1785,9 +1790,9 @@ static int pci_cfg_ok(struct domain *d, if ( ro_map && test_bit(machine_bdf, ro_map) ) return 0; } - start = d->arch.pci_cf8 & 0xFF; + start |= CF8_ADDR_LO(d->arch.pci_cf8); /* AMD extended configuration space access? */ - if ( (d->arch.pci_cf8 & 0x0F000000) && + if ( CF8_ADDR_HI(d->arch.pci_cf8) && boot_cpu_data.x86_vendor == X86_VENDOR_AMD && boot_cpu_data.x86 >= 0x10 && boot_cpu_data.x86 <= 0x17 ) { @@ -1796,7 +1801,7 @@ static int pci_cfg_ok(struct domain *d, if ( rdmsr_safe(MSR_AMD64_NB_CFG, msr_val) ) return 0; if ( msr_val & (1ULL << AMD64_NB_CFG_CF8_EXT_ENABLE_BIT) ) - start |= (d->arch.pci_cf8 >> 16) & 0xF00; + start |= CF8_ADDR_HI(d->arch.pci_cf8); } end = start + size - 1; if (xsm_pci_config_permission(XSM_HOOK, d, machine_bdf, start, end, write)) @@ -1855,7 +1860,7 @@ uint32_t guest_io_read( size = min(bytes, 4 - (port & 3)); if ( size == 3 ) size = 2; - if ( pci_cfg_ok(v->domain, 0, size) ) + if ( pci_cfg_ok(v->domain, 0, port & 3, size) ) sub_data = pci_conf_read(v->domain->arch.pci_cf8, port & 3, size); } @@ -1928,7 +1933,7 @@ void guest_io_write( size = min(bytes, 4 - (port & 3)); if ( size == 3 ) size = 2; - if ( pci_cfg_ok(v->domain, 1, size) ) + if ( pci_cfg_ok(v->domain, 1, port & 3, size) ) pci_conf_write(v->domain->arch.pci_cf8, port & 3, size, data); } --- a/xen/include/asm-x86/pci.h +++ b/xen/include/asm-x86/pci.h @@ -1,6 +1,11 @@ #ifndef __X86_PCI_H__ #define __X86_PCI_H__ +#define CF8_BDF(cf8) (((cf8) & 0x00ffff00) >> 8) +#define CF8_ADDR_LO(cf8) ((cf8) & 0x000000fc) +#define CF8_ADDR_HI(cf8) (((cf8) & 0x0f000000) >> 16) +#define CF8_ENABLED(cf8) (!!((cf8) & 0x80000000)) + #define IS_SNB_GFX(id) (id == 0x01068086 || id == 0x01168086 \ || id == 0x01268086 || id == 0x01028086 \ || id == 0x01128086 || id == 0x01228086 \