Re: [Virtio-fs] [PATCH 04/19] virtio: Implement get_shm_region for PCI transport

2019-08-27 Thread piaojun



On 2019/8/26 21:06, Vivek Goyal wrote:
> On Mon, Aug 26, 2019 at 09:43:08AM +0800, piaojun wrote:
> 
> [..]
>>> +static bool vp_get_shm_region(struct virtio_device *vdev,
>>> + struct virtio_shm_region *region, u8 id)
>>> +{
>>> +   struct virtio_pci_device *vp_dev = to_vp_device(vdev);
>>> +   struct pci_dev *pci_dev = vp_dev->pci_dev;
>>> +   u8 bar;
>>> +   u64 offset, len;
>>> +   phys_addr_t phys_addr;
>>> +   size_t bar_len;
>>> +   char *bar_name;
>>
>> 'char *bar_name' should be cleaned up to avoid compiling warning. And I
>> wonder if you mix tab and blankspace for code indent? Or it's just my
>> email display problem?
> 
> Will get rid of now unused bar_name. 
> 
OK

> Generally git flags if there are tab/space issues. I did not see any. So
> if you see something, point it out and I will fix it.
> 

cohuck found the same indent problem and pointed them in another email.

Jun


Re: [Virtio-fs] [PATCH 04/19] virtio: Implement get_shm_region for PCI transport

2019-08-26 Thread Vivek Goyal
On Mon, Aug 26, 2019 at 09:43:08AM +0800, piaojun wrote:

[..]
> > +static bool vp_get_shm_region(struct virtio_device *vdev,
> > + struct virtio_shm_region *region, u8 id)
> > +{
> > +   struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > +   struct pci_dev *pci_dev = vp_dev->pci_dev;
> > +   u8 bar;
> > +   u64 offset, len;
> > +   phys_addr_t phys_addr;
> > +   size_t bar_len;
> > +   char *bar_name;
> 
> 'char *bar_name' should be cleaned up to avoid compiling warning. And I
> wonder if you mix tab and blankspace for code indent? Or it's just my
> email display problem?

Will get rid of now unused bar_name. 

Generally git flags if there are tab/space issues. I did not see any. So
if you see something, point it out and I will fix it.

Vivek


Re: [Virtio-fs] [PATCH 04/19] virtio: Implement get_shm_region for PCI transport

2019-08-25 Thread piaojun



On 2019/8/22 1:57, Vivek Goyal wrote:
> From: Sebastien Boeuf 
> 
> On PCI the shm regions are found using capability entries;
> find a region by searching for the capability.
> 
> Cc: k...@vger.kernel.org
> Signed-off-by: Sebastien Boeuf 
> Signed-off-by: Dr. David Alan Gilbert 
> Signed-off-by: kbuild test robot 
> ---
>  drivers/virtio/virtio_pci_modern.c | 108 +
>  include/uapi/linux/virtio_pci.h|  11 ++-
>  2 files changed, 118 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_pci_modern.c 
> b/drivers/virtio/virtio_pci_modern.c
> index 7abcc50838b8..1cdedd93f42a 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -443,6 +443,112 @@ static void del_vq(struct virtio_pci_vq_info *info)
>   vring_del_virtqueue(vq);
>  }
>  
> +static int virtio_pci_find_shm_cap(struct pci_dev *dev,
> +   u8 required_id,
> +   u8 *bar, u64 *offset, u64 *len)
> +{
> + int pos;
> +
> +for (pos = pci_find_capability(dev, PCI_CAP_ID_VNDR);
> + pos > 0;
> + pos = pci_find_next_capability(dev, pos, PCI_CAP_ID_VNDR)) {
> + u8 type, cap_len, id;
> +u32 tmp32;
> +u64 res_offset, res_length;
> +
> + pci_read_config_byte(dev, pos + offsetof(struct virtio_pci_cap,
> + cfg_type),
> + &type);
> +if (type != VIRTIO_PCI_CAP_SHARED_MEMORY_CFG)
> +continue;
> +
> + pci_read_config_byte(dev, pos + offsetof(struct virtio_pci_cap,
> + cap_len),
> + &cap_len);
> + if (cap_len != sizeof(struct virtio_pci_cap64)) {
> + printk(KERN_ERR "%s: shm cap with bad size offset: %d 
> size: %d\n",
> +   __func__, pos, cap_len);
> +continue;
> +}
> +
> + pci_read_config_byte(dev, pos + offsetof(struct virtio_pci_cap,
> + id),
> + &id);
> +if (id != required_id)
> +continue;
> +
> +/* Type, and ID match, looks good */
> +pci_read_config_byte(dev, pos + offsetof(struct 
> virtio_pci_cap,
> + bar),
> + bar);
> +
> +/* Read the lower 32bit of length and offset */
> +pci_read_config_dword(dev, pos + offsetof(struct 
> virtio_pci_cap, offset),
> +  &tmp32);
> +res_offset = tmp32;
> +pci_read_config_dword(dev, pos + offsetof(struct 
> virtio_pci_cap, length),
> +  &tmp32);
> +res_length = tmp32;
> +
> +/* and now the top half */
> +pci_read_config_dword(dev,
> +  pos + offsetof(struct virtio_pci_cap64,
> + offset_hi),
> +  &tmp32);
> +res_offset |= ((u64)tmp32) << 32;
> +pci_read_config_dword(dev,
> +  pos + offsetof(struct virtio_pci_cap64,
> + length_hi),
> +  &tmp32);
> +res_length |= ((u64)tmp32) << 32;
> +
> +*offset = res_offset;
> +*len = res_length;
> +
> +return pos;
> +}
> +return 0;
> +}
> +
> +static bool vp_get_shm_region(struct virtio_device *vdev,
> +   struct virtio_shm_region *region, u8 id)
> +{
> + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct pci_dev *pci_dev = vp_dev->pci_dev;
> + u8 bar;
> + u64 offset, len;
> + phys_addr_t phys_addr;
> + size_t bar_len;
> + char *bar_name;

'char *bar_name' should be cleaned up to avoid compiling warning. And I
wonder if you mix tab and blankspace for code indent? Or it's just my
email display problem?

Thanks,
Jun