Re: [PATCH v15 1/1] vfio/nvgrace-gpu: Add vfio pci variant module for grace hopper

From: Ankit Agrawal
Date: Tue Jan 09 2024 - 06:42:54 EST


I am working to get the code changes suggested in this thread to get
to the next version.

Moreover based on the other design related comments for BAR approach
and support for the enable/disable bit, comments follow.

> There are a lot of VMMs and OSs this needs to support so it must all
> be consistent. For better or worse the decision was taken for the vPCI
> spec to use BAR not ACPI, in part due to feedback from the broader VMM
> ecosystem, and informed by future product plans.
>
> So, if vfio does special regions then qemu and everyone else has to
> fix it to meet the spec.

Ack, I'm putting this into the commit log as a reason to go with the BAR
approach for future reference.

>>> I'd suggest we take a look at whether we need to continue to pursue
>>> honoring the memory enable bit for these BARs and make a conscious and
>>> documented decision if we choose to ignore it.
>>
>> Let's document it.
> Ok, I'd certainly appreciate more background around why it was chosen
> to expose the device differently than bare metal in the commit log and
> code comments and in particular why we're consciously choosing not to
> honor these specific PCI semantics. Presumably we'd remove the -EIO
> from the r/w path based on the memory enable state as well.

Ack, will add to the commit log and remove EIO.

>> unprogrammed BARs are ignored (ie. not exposed to userspace), so perhaps
>> as long as it can be guaranteed that an access with the address space
>> enable bit cleared cannot generate a system level fault, we actually
>> have no strong argument to strictly enforce the address space bits.
>
> This is what I think, yes.

Ack, so given the access does not cause system level fault, I'll not enforce
the enable/disable bit. Though I'll put this information in the commit log
for reference as well and the reason for this choice.