Re: [PATCH 06/24] DAX: virtio-fs: Add cache BAR

[Stefan Hajnoczi]

On Tue, Feb 09, 2021 at 07:02:06PM +0000, Dr. David Alan Gilbert (git) wrote:
> @@ -46,6 +51,26 @@ static void vhost_user_fs_pci_realize(VirtIOPCIProxy 
> *vpci_dev, Error **errp)
>      }
>  
>      qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
> +    cachesize = dev->vdev.conf.cache_size;
> +
> +    /*
> +     * The bar starts with the data/DAX cache
> +     * Others will be added later.
> +     */
> +    memory_region_init(&dev->cachebar, OBJECT(vpci_dev),
> +                       "vhost-fs-pci-cachebar", cachesize);

s/vhost-fs/vhost-user-fs/ for consistency. Only worth changing if you
respin.

> +    if (cachesize) {
> +        memory_region_add_subregion(&dev->cachebar, 0, &dev->vdev.cache);
> +        virtio_pci_add_shm_cap(vpci_dev, VIRTIO_FS_PCI_CACHE_BAR, 0, 
> cachesize,
> +                               VIRTIO_FS_SHMCAP_ID_CACHE);
> +    }
> +
> +    /* After 'realized' so the memory region exists */
> +    pci_register_bar(&vpci_dev->pci_dev, VIRTIO_FS_PCI_CACHE_BAR,
> +                     PCI_BASE_ADDRESS_SPACE_MEMORY |
> +                     PCI_BASE_ADDRESS_MEM_PREFETCH |
> +                     PCI_BASE_ADDRESS_MEM_TYPE_64,
> +                     &dev->cachebar);

Please include a comment explainig why it's okay to use BAR 2, which is
already used for the virtio-pci modern io bar (off by default):

    /*
     * virtio pci bar layout used by default.
     * subclasses can re-arrange things if needed.
     *
     *   region 0   --  virtio legacy io bar
     *   region 1   --  msi-x bar
     *   region 2   --  virtio modern io bar (off by default)
     *   region 4+5 --  virtio modern memory (64bit) bar
     *
     */

I guess the idea is that the io bar is available since it's off by
default. What happens if the io bar is enabled?

Should this bar registration should be conditional (only when cache size
is greater than 0)?

signature.asc
Description: PGP signature