linux-kernel - Re: [PATCH v5 1/1] vfio/nvgpu: Add vfio pci variant module for grace hopper

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BY5PR12MB3763F22DF104E2B3BC65C628B006A@BY5PR12MB3763.namprd12.prod.outlook.com>
Date:   Fri, 28 Jul 2023 04:36:05 +0000
From:   Ankit Agrawal <ankita@...dia.com>
To:     Alex Williamson <alex.williamson@...hat.com>
CC:     Jason Gunthorpe <jgg@...dia.com>,
        Aniket Agashe <aniketa@...dia.com>, Neo Jia <cjia@...dia.com>,
        Kirti Wankhede <kwankhede@...dia.com>,
        "Tarun Gupta (SW-GPU)" <targupta@...dia.com>,
        Vikram Sethi <vsethi@...dia.com>,
        Andy Currid <acurrid@...dia.com>,
        Alistair Popple <apopple@...dia.com>,
        John Hubbard <jhubbard@...dia.com>,
        Dan Williams <danw@...dia.com>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 1/1] vfio/nvgpu: Add vfio pci variant module for grace
 hopper


>> +static ssize_t nvgrace_gpu_vfio_pci_read(struct vfio_device *core_vdev,
>> +             char __user *buf, size_t count, loff_t *ppos)
>> +{
>> +     unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
>> +     struct nvgrace_gpu_vfio_pci_core_device *nvdev = container_of(
>> +             core_vdev, struct nvgrace_gpu_vfio_pci_core_device, core_device.vdev);
>> +     u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
>> +     u8 val = 0xFF;
>> +     size_t i;
>> +
>> +     /*
>> +      * Only the device memory present on the hardware is mapped, which may
>> +      * not be power-of-2 aligned. A read to the BAR2 region implies an
>> +      * access outside the available device memory on the hardware.
>> +      */
>
> This is not true, userspace has no requirement to only access BAR2 via
> mmap.  This should support reads from within the coherent memory area.

Just to confirm, the ask is to just update the comment to reflect the behavior,
right? (I missed to do that in this posting). Because we do redirect the call to
vfio_pci_core_read() here which will perform the read that is within the device
region. The read response to synthesize -1 is only for the range that is outside
the device memory region.

>> +     if ((index == VFIO_PCI_BAR2_REGION_INDEX) &&
>> +             (offset >= nvdev->mem_length)) {
>> +             for (i = 0; i < count; i++)
>> +                     if (copy_to_user(buf + i, &val, 1))
>> +                             return -EFAULT;
>> +             return count;
>> +     }
>> +
>> +     return vfio_pci_core_read(core_vdev, buf, count, ppos);
>> +
>> +}
>> +
>> +static ssize_t nvgrace_gpu_vfio_pci_write(struct vfio_device *core_vdev,
>> +             const char __user *buf, size_t count, loff_t *ppos)
>> +{
>> +     unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
>> +     struct nvgrace_gpu_vfio_pci_core_device *nvdev = container_of(
>> +             core_vdev, struct nvgrace_gpu_vfio_pci_core_device, core_device.vdev);
>> +     u64 offset = *ppos & VFIO_PCI_OFFSET_MASK;
>> +
>> +     /*
>> +      * Only the device memory present on the hardware is mapped, which may
>> +      * not be power-of-2 aligned. A write to the BAR2 region implies an
>> +      * access outside the available device memory on the hardware.
>> +      */
>
> Likewise this should support writes within the coherent memory area.
> Disabling mmap support in QEMU is useful for tracing device accesses.
> Thanks,
>
> Alex

Same comment as above.

> +     if ((index == VFIO_PCI_BAR2_REGION_INDEX) &&
> +             (offset >= nvdev->mem_length))
> +             return count;
> +
> +     return vfio_pci_core_write(core_vdev, buf, count, ppos);
> +}