On 11/3/21 08:41, Andrew Jones wrote:
On Wed, Nov 03, 2021 at 08:00:09AM +0100, Thomas Huth wrote:
Sorry for the late reply - still trying to get my Inbox under control again ...
On 27/08/2021 12.17, Pierre Morel wrote:
To be able to use different VIRTIO transport in the future we need
the initialisation entry call of the transport to be inside the
transport file and keep the VIRTIO level transport agnostic.
Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
---
lib/virtio-mmio.c | 2 +-
lib/virtio-mmio.h | 2 --
lib/virtio.c | 5 -----
3 files changed, 1 insertion(+), 8 deletions(-)
diff --git a/lib/virtio-mmio.c b/lib/virtio-mmio.c
index e5e8f660..fb8a86a3 100644
--- a/lib/virtio-mmio.c
+++ b/lib/virtio-mmio.c
@@ -173,7 +173,7 @@ static struct virtio_device *virtio_mmio_dt_bind(u32 devid)
return &vm_dev->vdev;
}
-struct virtio_device *virtio_mmio_bind(u32 devid)
+struct virtio_device *virtio_bind(u32 devid)
{
return virtio_mmio_dt_bind(devid);
}
diff --git a/lib/virtio-mmio.h b/lib/virtio-mmio.h
index 250f28a0..73ddbd23 100644
--- a/lib/virtio-mmio.h
+++ b/lib/virtio-mmio.h
@@ -60,6 +60,4 @@ struct virtio_mmio_device {
void *base;
};
-extern struct virtio_device *virtio_mmio_bind(u32 devid);
-
#endif /* _VIRTIO_MMIO_H_ */
diff --git a/lib/virtio.c b/lib/virtio.c
index 69054757..e10153b9 100644
--- a/lib/virtio.c
+++ b/lib/virtio.c
@@ -123,8 +123,3 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len)
return ret;
}
-
-struct virtio_device *virtio_bind(u32 devid)
-{
- return virtio_mmio_bind(devid);
-}
I agree that this needs to be improved somehow, but I'm not sure whether
moving the function to virtio-mmio.c is the right solution. I guess the
original idea was that virtio_bind() could cope with multiple transports,
i.e. when there is support for virtio-pci, it could choose between mmio and
pci on ARM, or between CCW and PCI on s390x.
That's right. If we wanted to use virtio-pci on ARM, then, after
implementing virtio_pci_bind(), we'd change this to
struct virtio_device *virtio_bind(u32 devid)
{
struct virtio_device *dev = virtio_mmio_bind(devid);
if (!dev)
dev = virtio_pci_bind(devid);
return dev;
}
Then, we'd use config selection logic in the test harness to decide how to
construct the QEMU command line in order to choose between mmio and pci.
OK, I understand.
So maybe this should rather get an "#if defined(__arm__) ||
defined(__aarch64__)" instead? Drew, what's your opinion here?
Yup, but I think I'd prefer we do it in the header, like below, and
then also implement something like the above for virtio_bind().
diff --git a/lib/virtio-mmio.h b/lib/virtio-mmio.h
index 250f28a0d300..a0a3bf827156 100644
--- a/lib/virtio-mmio.h
+++ b/lib/virtio-mmio.h
@@ -60,6 +60,13 @@ struct virtio_mmio_device {
void *base;
};
+#if defined(__arm__) || defined(__aarch64__)
extern struct virtio_device *virtio_mmio_bind(u32 devid);
+#else
+static inline struct virtio_device *virtio_mmio_bind(u32 devid)
+{
+ return NULL;
+}
+#endif
#endif /* _VIRTIO_MMIO_H_ */
Thanks,
drew
Thanks, I will modify accordingly.
Regards,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
