On 02.09.21 14:59, Bram Hooimeijer wrote:
> Dear Jailhouse community, 
> 
> We are currently running into an issue when using PCIe caps beyond 0x100, e.g.
> those in the extended config space.
> 
> We want to write to some address, say 0x480. We assign the PCI cap to the
> non-root cell as shown below.
> 
>> .pci_devices = {
>>         /* PCIDevice: 3b:00.0 */
>>         {
>>                 .type = JAILHOUSE_PCI_TYPE_DEVICE,
>>                 .iommu = 5,
>>                 .domain = 0x0,
>>                 .bdf = 0x3b00,
>>                 .bar_mask = {
>>                         0xff000000, 0xffffffff, 0xff000000,
>>                         0xffffffff, 0x00000000, 0x00000000,
>>                 },
>>                 .caps_start = 0,
>>                 .num_caps = 1,
>>                 .num_msi_vectors = 0,
>>                 .msi_64bits = 0,
>>                 .msi_maskable = 0,
>>                 .num_msix_vectors = 0,
>>                 .msix_region_size = 0x0,
>>                 .msix_address = 0x0,
>>         },
>> },
>> .pci_caps = {
>>         {
>>                 .id = PCI_EXT_CAP_ID_VNDR | JAILHOUSE_PCI_EXT_CAP,
>>                 .start = 0x480,
>>                 .len = 0x80,
>>                 .flags = JAILHOUSE_PCICAPS_WRITE
>>         },
>> },
> 
> If we now write to this address, we get: 
> 
>> FATAL: Invalid PCI config write, port: 0xcfc, size: 4, address port: 
>> 0x843b0080
> 
> Even though the address port looks correct. Diving into the hypervisor, the
> value of address in hypervisor/pci.c:pci_cfg_write_moderate is 0x80 instead of
> 0x480. Due to that, it cannot find the cap.
> 
> The same happens for reads, however, with no cap these are still performed
> apparently.
> 
> The wrong address seems to originate from 
> hypervisor/arch/x86/pci.c:x86_pci_config_handler, which has the following
> snippet:
> 
> address = (addr_port_val & PCI_ADDR_REGNUM_MASK) + port 
>               - PCI_REG_DATA_PORT;
> 
> The extended config space, ie bits [31:24] from addr_port_val are dropped 
> here. 
> Hence, no config access can be done from 0x100 onwards.
> I think this should be:
> 
> address = (addr_port_val & PCI_ADDR_REGNUM_MASK) + port 
>               - PCI_REG_DATA_PORT
>               + ((addr_port_val && BITMASK(31,24))>>16);
> 
> Is this a bug, or is this intentional and did I misconfigure the PCI extended
> capabilities? I can happily submit a patch, but given that PCIe usage is so 
> common, I expect I have misconfigured something.

This is indeed likely a bug. Just checked the Linux code
(arch/x86/pci/direct.c, PCI_CONF1_ADDRESS), and that also considers the
upper 4 bits for the extended config space.

I guess I was always under the assumption that only MMCONFIG would
permit access to extended config space, and all target configurations we
used to far actually showed that behaviour (likely because it's the
faster path anyway). Does your target have no MMCONFIG space? Would be
good to understand why only your system ran into this.

That said, we could also extend the access model in
hypervisor/arch/x86/pci.c, adding the upper bits consistently (3 places,
as it looks like). Maybe PCI_ADDR_REGNUM_MASK should be converted into
something that translates the register address into the address port value.

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

-- 
You received this message because you are subscribed to the Google Groups 
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jailhouse-dev/5e385f8b-5af0-444b-0a4d-9904534ed719%40siemens.com.

Reply via email to