On 12/9/24 5:09 PM, David Hildenbrand wrote:
> On 09.12.24 22:45, Matthew Rosato wrote:
>> On 12/9/24 4:01 PM, David Hildenbrand wrote:
>>> On 09.12.24 20:29, Matthew Rosato wrote:
>>>
>>> Hi,
>>>
>>> Trying to wrap my head around that ... you mention that "pin the entirety
>>> of guest memory".
>>>
>>> Do you mean that we will actually end up longterm pinning all guest RAM in
>>> the kernel, similar to what vfio ends up doing?
>>
>> Yes. Actually, the usecase here is specifically PCI passthrough via
>> vfio-pci on s390. Unlike other platforms, the default s390 approach only
>> pins on-demand and doesn't longterm pin all guest RAM, which is nice from a
>> memory footprint perspective but pays a price via all those guest-2 RPCIT
>> instructions. The goal here is now provide the optional alternative to
>> longterm pin like other platforms.
>
> Okay, thanks for confirming. One more question: who will trigger this
> longterm-pinning? Is it vfio?
>
> (the code flow from your code to the pinning code would be nice)
>
Yes, the vfio IOMMU code triggers it. My s390_pci_setup_stage2_map added by
this patch calls memory_region_notify_iommu in a loop such that we trigger
iommu notifications to map iova X+pba -> GPA X for all GPA, where pba = a "base
address" offset that has to be applied when mapping on s390. The notifications
are sent in the largest batches possible to minimize vfio ioctls / use the
least number of vfio dma mappings.
The iommu notifications get picked up in vfio_iommu_map_notify where we will
follow the container DMA path down to vfio_legacy_dma_map; then ultimately vfio
is handling the pinning via the VFIO_IOMMU_MAP_DMA ioctl.
>>
>>>
>>> In that case, it would be incompatible with virtio-balloon (and without
>>> modifications with upcoming virtio-mem). Is there already a mechanism in
>>> place to handle that -- a call to ram_block_discard_disable() -- or even a
>>> way to support coordinated discarding of RAM (e.g., virtio-mem + vfio)?
>>
>> Good point, should be calling add ram_block_discard_disable(true) when set
>> register + a corresponding (false) during deregister... Will add for v2.
>>
>> As for supporting coordinated discard, I was hoping to subsequently look at
>> virtio-mem for this.
>
> As long as discarding is blocked for now, we're good. To support it, the
> RAMDiscardManager would have to be wired up, similar to vfio.
>
> I think the current way of handling it via
> vf
> + IOMMUTLBEvent event = {
> + .type = IOMMU_NOTIFIER_MAP,
> + .entry = {
> + .target_as = &address_space_memory,
> + .translated_addr = 0,
> + .perm = IOMMU_RW,
> + },
> + };
>
>
> Is probably not ideal: it cannot cope with memory holes (which virtio-mem
> would create).
>
> Likely, you'd instead want an address space notifier, and really only map the
> memory region sections you get notified about.
>
> There, you can test for RAMDiscardManager and handle it like vfio does.
>
I'll start looking into this; for the moment I'll plan on blocking discarding
in this series with a follow-on to then enable virtio-mem, but if I get
something working sooner I'll add it to this series. Either way I'll put you
on CC.
