On 2018-01-18 00:00, Gustavo Lima Chaves wrote:
> Try our best to follow more closely what the specification mandates for
> function-level reset (section 6.6.2 of PCI Express Base Specification,
> Revision 3.0). The previous approach was writing blindly on the command
> register (setting int. disable bit), but that would lead to an
> unsupported request on functions that do not expect that right away.
>
> We now do checks for FLR support, proceed to the previous command
> register write, wait for any pending transactions for some cycles (used
> as a placeholder for the suggested 100ms timeout), raise the 'initiate
> FLR' bit on the right register, wait on the same kind of loop again and,
> finally, clear the transactions pending bit forcefully, as mandated.
>
> This should leave us with better support for PCI devices out there.
>
> Signed-off-by: Gustavo Lima Chaves <[email protected]>
> ---
> hypervisor/include/jailhouse/pci.h | 6 +++
> hypervisor/pci.c | 77
> +++++++++++++++++++++++++++++++++++---
> include/jailhouse/cell-config.h | 2 +
> 3 files changed, 80 insertions(+), 5 deletions(-)
>
> diff --git a/hypervisor/include/jailhouse/pci.h
> b/hypervisor/include/jailhouse/pci.h
> index bed1498a..4befef11 100644
> --- a/hypervisor/include/jailhouse/pci.h
> +++ b/hypervisor/include/jailhouse/pci.h
> @@ -40,7 +40,13 @@
> #define PCI_CAP_EXPRESS 0x10
>
> #define PCI_CAP_PCIE 0x10
> +#define PCIE_DEV_CAPS_REG 0x04
> +# define PCI_DEV_CAPS_FLR (1 << 28)
> +
> #define PCIE_CONTROL_REG 0x08
> +# define PCI_CONTROL_INIT_FLR (1 << 15)
> +#define PCIE_STATUS_REG 0x0a
> +# define PCI_STATUS_PENDING (1 << 5)
>
> #define PCIE_DEVCTL_CERE 0x0001 /* Correctable Error Reporting En. */
> #define PCIE_DEVCTL_NFERE 0x0002 /* Non-Fatal Error Reporting Enable */
> diff --git a/hypervisor/pci.c b/hypervisor/pci.c
> index 00ba041f..9ee9dee4 100644
> --- a/hypervisor/pci.c
> +++ b/hypervisor/pci.c
> @@ -610,10 +610,12 @@ void pci_prepare_handover(void)
> }
> }
>
> -void pci_reset_device(struct pci_device *device)
> +static void pci_reset_device_internal(struct pci_device *device)
> {
> const struct jailhouse_pci_capability *cap;
> + unsigned timeout = device->info->pci_flr_timeout;
> unsigned int n;
> + u32 val;
>
> memset(&device->msi_registers, 0, sizeof(device->msi_registers));
> for (n = 0; n < device->info->num_msix_vectors; n++) {
> @@ -627,13 +629,79 @@ void pci_reset_device(struct pci_device *device)
> return;
> }
>
> + cap = pci_get_cap(device, PCI_CAP_PCIE | JAILHOUSE_PCI_EXT_CAP);
> + if (!cap)
> + return;
> +
> + val = pci_read_config(device->info->bdf,
Let's cache bdf in local var, will give us shorter statements.
> + cap->start + PCIE_DEV_CAPS_REG, 4);
> +
> + if (!(val & PCI_DEV_CAPS_FLR)) {
> + printk("Skipping function level reset for "
> + "device %02x:%02x.%x: no support\n",
> + PCI_BDF_PARAMS(device->info->bdf));
Why warn here, but not if the device isn't PCIe-capable?
> + return;
If FLR is not supported, we should still do the silencing of the device
via the command register below. Just move the command reg update before
all the FLR-related logic.
> + }
> +
> /*
> - * Silence INTx of the physical device by setting the mask bit.
> - * This is a deviation from the specified reset state.
> + * "When a 0 is written to this register, the device is
> + * logically disconnected from the PCI bus for all accesses
> + * except configuration accesses."
> + *
> + * The bit that we set on that mask will silence INTx of the
> + * physical device.
> */
> pci_write_config(device->info->bdf, PCI_CFG_COMMAND,
> PCI_CMD_INTX_OFF, 2);
>
> + /*
> + * Wait for any pending transaction to end. Ideally we should
> + * count for 100ms, but we count arbitrary loops for now
> + * instead, by lack of timer notion in the hypervisor.
> + */
> + do {
> + val = pci_read_config(device->info->bdf,
> + cap->start + PCIE_STATUS_REG, 2);
> + cpu_relax();
> + } while ((val & PCI_STATUS_PENDING)
> + || (device->info->pci_flr_timeout ? --timeout : false));
> +
> + /*
> + * Set "initiate function level reset" bit and wait a little
> + * bit more.
> + */
> + val = pci_read_config(device->info->bdf,
> + cap->start + PCIE_CONTROL_REG, 2);
> + pci_write_config(device->info->bdf, cap->start + PCIE_CONTROL_REG,
> + val & PCI_CONTROL_INIT_FLR, 2);
> +
> + timeout = device->info->pci_flr_timeout;
> + do {
> + /*
> + * Read a possibly new status reg value for the write
> + * below, also making the two timing loops equal in time
> + * spans.
> + */
> + val = pci_read_config(device->info->bdf,
> + cap->start + PCIE_STATUS_REG, 2);
> + cpu_relax();
> + } while (device->info->pci_flr_timeout ? --timeout : false);
> +
> + /*
> + * "The Transactions Pending bit must be cleared upon completion
> + * of the FLR."
> + */
> + pci_write_config(device->info->bdf, cap->start + PCIE_STATUS_REG,
> + val & ~PCI_STATUS_PENDING, 2);
> +}
> +
> +void pci_reset_device(struct pci_device *device)
> +{
> + const struct jailhouse_pci_capability *cap;
> + unsigned int n;
> +
> + pci_reset_device_internal(device);
> +
> for_each_pci_cap(cap, device, n) {
> if (cap->id == PCI_CAP_MSI || cap->id == PCI_CAP_MSIX)
> /* Disable MSI/MSI-X by clearing the control word. */
> @@ -929,7 +997,6 @@ void pci_shutdown(void)
> pci_restore_msix(device, cap);
>
> if (device->cell != &root_cell)
> - pci_write_config(device->info->bdf, PCI_CFG_COMMAND,
> - PCI_CMD_INTX_OFF, 2);
> + pci_reset_device_internal(device);
> }
> }
> diff --git a/include/jailhouse/cell-config.h b/include/jailhouse/cell-config.h
> index cf50ef84..72c4facc 100644
> --- a/include/jailhouse/cell-config.h
> +++ b/include/jailhouse/cell-config.h
> @@ -141,6 +141,8 @@ struct jailhouse_pci_device {
> __u16 domain;
> __u16 bdf;
> __u32 bar_mask[6];
> + /** A cycle-count based value that Jailhouse will wait for the device
> to terminate any PCI transactions during function-level reset. This *won't*
> be filled up by jailhouse-config-create tool automatically, so tailor this to
> your devices and system. */
Overlong line.
And we do need a reasonable default, set by config-create. So drop that
part of the comment.
> + __u32 pci_flr_timeout;
> __u16 caps_start;
> __u16 num_caps;
> __u8 num_msi_vectors;
>
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
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].
For more options, visit https://groups.google.com/d/optout.
