On 23.03.2023 11:26, Huang Rui wrote: > On Wed, Mar 22, 2023 at 08:48:30PM +0800, Jan Beulich wrote: >> On 22.03.2023 13:33, Huang Rui wrote: >>> I traced that while we do pci-assignable-add, we will follow below trace to >>> bind the passthrough device. >>> >>> pciassignable_add()->libxl_device_pci_assignable_add()->libxl__device_pci_assignable_add()->pciback_dev_assign() >>> >>> Then kernel xen-pciback driver want to add virtual configuration spaces. In >>> this phase, the bar_write() in xen hypervisor will be called. I still need >>> a bit more time to figure the exact reason. May I know where the >>> xen-pciback driver would trigger a hvm_io_intercept to xen hypervisor? >> >> Any config space access would. And I might guess ... >> >>> [ 309.719049] xen_pciback: wants to seize 0000:03:00.0 >>> [ 462.911251] pciback 0000:03:00.0: xen_pciback: probing... >>> [ 462.911256] pciback 0000:03:00.0: xen_pciback: seizing device >>> [ 462.911257] pciback 0000:03:00.0: xen_pciback: pcistub_device_alloc >>> [ 462.911261] pciback 0000:03:00.0: xen_pciback: initializing... >>> [ 462.911263] pciback 0000:03:00.0: xen_pciback: initializing config >>> [ 462.911265] pciback 0000:03:00.0: xen-pciback: initializing virtual >>> configuration space >>> [ 462.911268] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0x00 >>> [ 462.911271] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0x02 >>> [ 462.911284] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0x04 >>> [ 462.911286] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0x3c >>> [ 462.911289] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0x3d >>> [ 462.911291] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0x0c >>> [ 462.911294] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0x0d >>> [ 462.911296] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0x0f >>> [ 462.911301] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0x10 >>> [ 462.911306] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0x14 >>> [ 462.911309] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0x18 >>> [ 462.911313] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0x1c >>> [ 462.911317] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0x20 >>> [ 462.911321] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0x24 >>> [ 462.911325] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0x30 >>> [ 462.911358] pciback 0000:03:00.0: Found capability 0x1 at 0x50 >>> [ 462.911361] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0x50 >>> [ 462.911363] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0x52 >>> [ 462.911368] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0x54 >>> [ 462.911371] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0x56 >>> [ 462.911373] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0x57 >>> [ 462.911386] pciback 0000:03:00.0: Found capability 0x5 at 0xa0 >>> [ 462.911388] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0xa0 >>> [ 462.911391] pciback 0000:03:00.0: xen-pciback: added config field at >>> offset 0xa2 >>> [ 462.911405] pciback 0000:03:00.0: xen_pciback: enabling device >>> [ 462.911412] pciback 0000:03:00.0: enabling device (0006 -> 0007) >>> [ 462.911658] Already setup the GSI :28 >>> [ 462.911668] Already map the GSI :28 and IRQ: 115 >>> [ 462.911684] pciback 0000:03:00.0: xen_pciback: save state of device >>> [ 462.912154] pciback 0000:03:00.0: xen_pciback: resetting (FLR, D3, etc) >>> the device >>> [ 463.954998] pciback 0000:03:00.0: xen_pciback: reset device >> >> ... it is actually the reset here, saving and then restoring config space. >> If e.g. that restore was done "blindly" (i.e. simply writing fields low to >> high), then memory decode would be re-enabled before the BARs are written. >> > > Yes, we confirm the problem is while the xen-pciback driver initializes > passthrough device with pcistub_init_device() -> pci_restore_state() -> > pci_restore_config_space() -> pci_restore_config_space_range() -> > pci_restore_config_dword() -> pci_write_config_dword(), the pci config > write will trigger io interrupt to bar_write() in the xen, then bar->enable > is set, the write is not actually allowed. > > May I know whether this behavior (restore) is expected? Or it should not > reset the device.
The reset is expected. To expand slightly on Roger's reply: The reset we're unaware of has likely indeed brought bar->enable and command register state out of sync. For everything else see Roger's response. Jan
