net/virtio: fix secondary process crash with PCI devices

The Virtio rework series mistakenly moved the rte_pci_device
pointer to struct virtio_hw, which is shared between the two
processes. But this structure is per-process, so this change
made secondary process to try accessing primary process-only
memory, leading to a crash.

This patch reverts to proper behavior, by storing the
rte_pci_device pointer into the per-process
virtio_pci_internal struct. It also provides helper to get
the pointer from the virtio_hw struct pointer.

Bugzilla ID: 633
Fixes: c8d4b02f72ae ("net/virtio: move legacy IO to virtio PCI")

Reported-by: Anatoly Burakov <anatoly.burakov@intel.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
This commit is contained in:
Maxime Coquelin 2021-02-03 16:58:11 +01:00 committed by Ferruh Yigit
parent fa545ed3da
commit aa0d4b8a01
3 changed files with 18 additions and 21 deletions

View File

@ -31,13 +31,6 @@
#define VIRTIO_PCI_CONFIG(dev) \
(((dev)->msix_status == VIRTIO_MSIX_ENABLED) ? 24 : 20)
struct virtio_pci_internal {
struct rte_pci_ioport io;
};
#define VTPCI_IO(hw) (&virtio_pci_internal[(hw)->port_id].io)
struct virtio_pci_internal virtio_pci_internal[RTE_MAX_ETHPORTS];
static inline int
@ -313,16 +306,14 @@ legacy_intr_detect(struct virtio_hw *hw)
{
struct virtio_pci_dev *dev = virtio_pci_get_dev(hw);
dev->msix_status = vtpci_msix_detect(dev->pci_dev);
dev->msix_status = vtpci_msix_detect(VTPCI_DEV(hw));
hw->intr_lsc = !!dev->msix_status;
}
static int
legacy_dev_close(struct virtio_hw *hw)
{
struct virtio_pci_dev *dev = virtio_pci_get_dev(hw);
rte_pci_unmap_device(dev->pci_dev);
rte_pci_unmap_device(VTPCI_DEV(hw));
rte_pci_ioport_unmap(VTPCI_IO(hw));
return 0;
@ -574,16 +565,14 @@ modern_intr_detect(struct virtio_hw *hw)
{
struct virtio_pci_dev *dev = virtio_pci_get_dev(hw);
dev->msix_status = vtpci_msix_detect(dev->pci_dev);
dev->msix_status = vtpci_msix_detect(VTPCI_DEV(hw));
hw->intr_lsc = !!dev->msix_status;
}
static int
modern_dev_close(struct virtio_hw *hw)
{
struct virtio_pci_dev *dev = virtio_pci_get_dev(hw);
rte_pci_unmap_device(dev->pci_dev);
rte_pci_unmap_device(VTPCI_DEV(hw));
return 0;
}
@ -772,8 +761,6 @@ vtpci_init(struct rte_pci_device *pci_dev, struct virtio_pci_dev *dev)
RTE_BUILD_BUG_ON(offsetof(struct virtio_pci_dev, hw) != 0);
dev->pci_dev = pci_dev;
/*
* Try if we can succeed reading virtio pci caps, which exists
* only on modern pci device. If failed, we fallback to legacy
@ -816,7 +803,5 @@ void vtpci_legacy_ioport_unmap(struct virtio_hw *hw)
int vtpci_legacy_ioport_map(struct virtio_hw *hw)
{
struct virtio_pci_dev *dev = virtio_pci_get_dev(hw);
return rte_pci_ioport_map(dev->pci_dev, 0, VTPCI_IO(hw));
return rte_pci_ioport_map(VTPCI_DEV(hw), 0, VTPCI_IO(hw));
}

View File

@ -104,7 +104,6 @@ enum virtio_msix_status {
struct virtio_pci_dev {
struct virtio_hw hw;
struct rte_pci_device *pci_dev;
struct virtio_pci_common_cfg *common_cfg;
struct virtio_net_config *dev_cfg;
enum virtio_msix_status msix_status;
@ -116,6 +115,17 @@ struct virtio_pci_dev {
#define virtio_pci_get_dev(hwp) container_of(hwp, struct virtio_pci_dev, hw)
struct virtio_pci_internal {
struct rte_pci_ioport io;
struct rte_pci_device *dev;
};
extern struct virtio_pci_internal virtio_pci_internal[RTE_MAX_ETHPORTS];
#define VTPCI_IO(hw) (&virtio_pci_internal[(hw)->port_id].io)
#define VTPCI_DEV(hw) (virtio_pci_internal[(hw)->port_id].dev)
/*
* How many bits to shift physical queue address written to QUEUE_PFN.
* 12 is historical, and due to x86 page size.

View File

@ -78,12 +78,14 @@ eth_virtio_pci_init(struct rte_eth_dev *eth_dev)
if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
hw->port_id = eth_dev->data->port_id;
VTPCI_DEV(hw) = pci_dev;
ret = vtpci_init(RTE_ETH_DEV_TO_PCI(eth_dev), dev);
if (ret) {
PMD_INIT_LOG(ERR, "Failed to init PCI device\n");
return -1;
}
} else {
VTPCI_DEV(hw) = pci_dev;
if (dev->modern)
VIRTIO_OPS(hw) = &modern_ops;
else