> On 4 Feb 2022, at 16.12, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Thu, Feb 03, 2022 at 05:12:31AM -0800, Jorgen Hansen wrote:
>> Detect the support for MMIO access through examination of the length
>> of the region requested in BAR1. If it is 256KB, the VMCI device
>> supports MMIO access to registers.
>>
>> If MMIO access is supported, map the area of the region used for
>> MMIO access (64KB size at offset 128KB).
>>
>> Add wrapper functions for accessing 32 bit register accesses through
>> either MMIO or IO ports based on device configuration.
>>
>> Sending and receiving datagrams through iowrite8_rep/ioread8_rep is
>> left unchanged for now, and will be addressed in a later change.
>>
>> Reviewed-by: Vishnu Dasa <vdasa@xxxxxxxxxx>
>> Signed-off-by: Jorgen Hansen <jhansen@xxxxxxxxxx>
>> ---
>> drivers/misc/vmw_vmci/vmci_guest.c | 68 ++++++++++++++++++++++--------
>> include/linux/vmw_vmci_defs.h | 12 ++++++
>> 2 files changed, 62 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/misc/vmw_vmci/vmci_guest.c b/drivers/misc/vmw_vmci/vmci_guest.c
>> index 1018dc77269d..38ee7ed32ab9 100644
>> --- a/drivers/misc/vmw_vmci/vmci_guest.c
>> +++ b/drivers/misc/vmw_vmci/vmci_guest.c
>> @@ -45,6 +45,7 @@ static u32 vm_context_id = VMCI_INVALID_ID;
>> struct vmci_guest_device {
>> struct device *dev; /* PCI device we are attached to */
>> void __iomem *iobase;
>> + void __iomem *mmio_base;
>>
>> bool exclusive_vectors;
>>
>> @@ -89,6 +90,21 @@ u32 vmci_get_vm_context_id(void)
>> return vm_context_id;
>> }
>>
>> +static unsigned int vmci_read_reg(struct vmci_guest_device *dev, u32 reg)
>> +{
>> + if (dev->mmio_base != NULL)
>> + return readl(dev->mmio_base + reg);
>> + return ioread32(dev->iobase + reg);
>> +}
>> +
>> +static void vmci_write_reg(struct vmci_guest_device *dev, u32 val, u32 reg)
>> +{
>> + if (dev->mmio_base != NULL)
>> + writel(val, dev->mmio_base + reg);
>> + else
>> + iowrite32(val, dev->iobase + reg);
>> +}
>> +
>> /*
>> * VM to hypervisor call mechanism. We use the standard VMware naming
>> * convention since shared code is calling this function as well.
>> @@ -116,7 +132,7 @@ int vmci_send_datagram(struct vmci_datagram *dg)
>> if (vmci_dev_g) {
>> iowrite8_rep(vmci_dev_g->iobase + VMCI_DATA_OUT_ADDR,
>> dg, VMCI_DG_SIZE(dg));
>> - result = ioread32(vmci_dev_g->iobase + VMCI_RESULT_LOW_ADDR);
>> + result = vmci_read_reg(vmci_dev_g, VMCI_RESULT_LOW_ADDR);
>> } else {
>> result = VMCI_ERROR_UNAVAILABLE;
>> }
>> @@ -384,7 +400,7 @@ static irqreturn_t vmci_interrupt(int irq, void *_dev)
>> unsigned int icr;
>>
>> /* Acknowledge interrupt and determine what needs doing. */
>> - icr = ioread32(dev->iobase + VMCI_ICR_ADDR);
>> + icr = vmci_read_reg(dev, VMCI_ICR_ADDR);
>> if (icr == 0 || icr == ~0)
>> return IRQ_NONE;
>>
>> @@ -429,7 +445,8 @@ static int vmci_guest_probe_device(struct pci_dev *pdev,
>> const struct pci_device_id *id)
>> {
>> struct vmci_guest_device *vmci_dev;
>> - void __iomem *iobase;
>> + void __iomem *iobase = NULL;
>> + void __iomem *mmio_base = NULL;
>> unsigned int capabilities;
>> unsigned int caps_in_use;
>> unsigned long cmd;
>> @@ -445,16 +462,32 @@ static int vmci_guest_probe_device(struct pci_dev *pdev,
>> return error;
>> }
>>
>> - error = pcim_iomap_regions(pdev, 1 << 0, KBUILD_MODNAME);
>> - if (error) {
>> - dev_err(&pdev->dev, "Failed to reserve/map IO regions\n");
>> - return error;
>> + /*
>> + * The VMCI device with mmio access to registers requests 256KB
>> + * for BAR1. If present, driver will use new VMCI device
>> + * functionality for register access and datagram send/recv.
>> + */
>> +
>> + if (pci_resource_len(pdev, 1) == VMCI_WITH_MMIO_ACCESS_BAR_SIZE) {
>> + dev_info(&pdev->dev, "MMIO register access is available\n");
>> + mmio_base = pci_iomap_range(pdev, 1, VMCI_MMIO_ACCESS_OFFSET,
>> + VMCI_MMIO_ACCESS_SIZE);
>> + /* If the map fails, we fall back to IOIO access. */
>> + if (!mmio_base)
>> + dev_warn(&pdev->dev, "Failed to map MMIO register access\n");
>> }
>>
>> - iobase = pcim_iomap_table(pdev)[0];
>> + if (!mmio_base) {
>> + error = pcim_iomap_regions(pdev, BIT(0), KBUILD_MODNAME);
>> + if (error) {
>> + dev_err(&pdev->dev, "Failed to reserve/map IO regions\n");
>> + return error;
>> + }
>> + iobase = pcim_iomap_table(pdev)[0];
>> + }
>>
>> - dev_info(&pdev->dev, "Found VMCI PCI device at %#lx, irq %u\n",
>> - (unsigned long)iobase, pdev->irq);
>> + dev_info(&pdev->dev, "Found VMCI PCI device at %#lx, %#lx, irq %u\n",
>> + (unsigned long)iobase, (unsigned long)mmio_base, pdev->irq);
>
> Why are you printing iomem addresses to userspace? Are you sure that is
> ok? Who is going to use it?
I’m not sure if there was a specific purpose back when this was added - most
likely the intention was simply to report the resources allocated to the device.
I'll remove this one too.
Thanks,
Jorgen
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
