diff mbox series

[V1,5/6] xen/grant-dma-ops: Retrieve the ID of backend's domain for DT devices

Message ID 1650646263-22047-6-git-send-email-olekstysh@gmail.com (mailing list archive)
State Superseded
Headers show
Series virtio: Solution to restrict memory access under Xen using xen-grant DMA-mapping layer | expand

Commit Message

Oleksandr Tyshchenko April 22, 2022, 4:51 p.m. UTC
From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

Use the presence of recently introduced "xen,dev-domid" property
in the device node as a clear indicator of enabling Xen grant
mappings scheme for that device and read the ID of Xen domain where
the corresponding backend resides. The ID (domid) is used as
an argument to the Xen grant mapping APIs.

Also introduce xen_is_grant_dma_device() to check whether xen-grant
DMA ops need to be set for a passed device.

Remove the hardcoded domid 0 in xen_grant_setup_dma_ops().

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
---
Changes RFC -> V1:
   - new patch, split required changes from commit:
    "[PATCH 4/6] virtio: Various updates to xen-virtio DMA ops layer"
   - update checks in xen_virtio_setup_dma_ops() to only support
     DT devices for now
   - remove the "virtio,mmio" check from xen_is_virtio_device()
   - remane everything according to the new naming scheme:
     s/virtio/grant_dma
---
 drivers/xen/grant-dma-ops.c | 25 ++++++++++++++++++-------
 include/xen/xen-ops.h       |  5 +++++
 2 files changed, 23 insertions(+), 7 deletions(-)

Comments

Stefano Stabellini April 22, 2022, 11 p.m. UTC | #1
On Fri, 22 Apr 2022, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> 
> Use the presence of recently introduced "xen,dev-domid" property
> in the device node as a clear indicator of enabling Xen grant
> mappings scheme for that device and read the ID of Xen domain where
> the corresponding backend resides. The ID (domid) is used as
> an argument to the Xen grant mapping APIs.
> 
> Also introduce xen_is_grant_dma_device() to check whether xen-grant
> DMA ops need to be set for a passed device.
> 
> Remove the hardcoded domid 0 in xen_grant_setup_dma_ops().
> 
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> ---
> Changes RFC -> V1:
>    - new patch, split required changes from commit:
>     "[PATCH 4/6] virtio: Various updates to xen-virtio DMA ops layer"
>    - update checks in xen_virtio_setup_dma_ops() to only support
>      DT devices for now
>    - remove the "virtio,mmio" check from xen_is_virtio_device()
>    - remane everything according to the new naming scheme:
>      s/virtio/grant_dma
> ---
>  drivers/xen/grant-dma-ops.c | 25 ++++++++++++++++++-------
>  include/xen/xen-ops.h       |  5 +++++
>  2 files changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c
> index 0e69aa8..70d5d77 100644
> --- a/drivers/xen/grant-dma-ops.c
> +++ b/drivers/xen/grant-dma-ops.c
> @@ -66,11 +66,6 @@ static struct xen_grant_dma_data *find_xen_grant_dma_data(struct device *dev)
>   * Such a DMA address is formed by using the grant reference as a frame
>   * number and setting the highest address bit (this bit is for the backend
>   * to be able to distinguish it from e.g. a mmio address).
> - *
> - * Note that for now we hard wire dom0 to be the backend domain. In order
> - * to support any domain as backend we'd need to add a way to communicate
> - * the domid of this backend, e.g. via Xenstore, via the PCI-device's
> - * config space or DT/ACPI.
>   */
>  static void *xen_grant_dma_alloc(struct device *dev, size_t size,
>  				 dma_addr_t *dma_handle, gfp_t gfp,
> @@ -277,6 +272,16 @@ static const struct dma_map_ops xen_grant_dma_ops = {
>  	.dma_supported = xen_grant_dma_supported,
>  };
>  
> +bool xen_is_grant_dma_device(struct device *dev)
> +{
> +	/* XXX Handle only DT devices for now */
> +	if (!dev->of_node)
> +		return false;
> +
> +	return of_property_read_bool(dev->of_node, "xen,dev-domid");
> +}
> +EXPORT_SYMBOL_GPL(xen_is_grant_dma_device);
> +
>  void xen_grant_setup_dma_ops(struct device *dev)
>  {
>  	struct xen_grant_dma_data *data;
> @@ -288,8 +293,14 @@ void xen_grant_setup_dma_ops(struct device *dev)
>  		return;
>  	}
>  
> -	/* XXX The dom0 is hardcoded as the backend domain for now */
> -	dev_domid = 0;
> +	/* XXX ACPI and PCI devices unsupported for now */
> +	if (dev_is_pci(dev) || !dev->of_node)
> +		goto err;

I think we can remove the "dev_is_pci" check, right?


> +	if (of_property_read_u32(dev->of_node, "xen,dev-domid", &dev_domid)) {
> +		dev_err(dev, "xen,dev-domid property is not present\n");
> +		goto err;
> +	}
>  
>  	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>  	if (!data) {
> diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
> index 4f9fad5..62be9dc 100644
> --- a/include/xen/xen-ops.h
> +++ b/include/xen/xen-ops.h
> @@ -223,10 +223,15 @@ static inline void xen_preemptible_hcall_end(void) { }
>  
>  #ifdef CONFIG_XEN_GRANT_DMA_OPS
>  void xen_grant_setup_dma_ops(struct device *dev);
> +bool xen_is_grant_dma_device(struct device *dev);
>  #else
>  static inline void xen_grant_setup_dma_ops(struct device *dev)
>  {
>  }
> +static inline bool xen_is_grant_dma_device(struct device *dev)
> +{
> +	return false;
> +}
>  #endif /* CONFIG_XEN_GRANT_DMA_OPS */
>  
>  #endif /* INCLUDE_XEN_OPS_H */
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Oleksandr Tyshchenko April 23, 2022, 3:23 p.m. UTC | #2
On 23.04.22 02:00, Stefano Stabellini wrote:

Hello Stefano

> On Fri, 22 Apr 2022, Oleksandr Tyshchenko wrote:
>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>
>> Use the presence of recently introduced "xen,dev-domid" property
>> in the device node as a clear indicator of enabling Xen grant
>> mappings scheme for that device and read the ID of Xen domain where
>> the corresponding backend resides. The ID (domid) is used as
>> an argument to the Xen grant mapping APIs.
>>
>> Also introduce xen_is_grant_dma_device() to check whether xen-grant
>> DMA ops need to be set for a passed device.
>>
>> Remove the hardcoded domid 0 in xen_grant_setup_dma_ops().
>>
>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>> ---
>> Changes RFC -> V1:
>>     - new patch, split required changes from commit:
>>      "[PATCH 4/6] virtio: Various updates to xen-virtio DMA ops layer"
>>     - update checks in xen_virtio_setup_dma_ops() to only support
>>       DT devices for now
>>     - remove the "virtio,mmio" check from xen_is_virtio_device()
>>     - remane everything according to the new naming scheme:
>>       s/virtio/grant_dma
>> ---
>>   drivers/xen/grant-dma-ops.c | 25 ++++++++++++++++++-------
>>   include/xen/xen-ops.h       |  5 +++++
>>   2 files changed, 23 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c
>> index 0e69aa8..70d5d77 100644
>> --- a/drivers/xen/grant-dma-ops.c
>> +++ b/drivers/xen/grant-dma-ops.c
>> @@ -66,11 +66,6 @@ static struct xen_grant_dma_data *find_xen_grant_dma_data(struct device *dev)
>>    * Such a DMA address is formed by using the grant reference as a frame
>>    * number and setting the highest address bit (this bit is for the backend
>>    * to be able to distinguish it from e.g. a mmio address).
>> - *
>> - * Note that for now we hard wire dom0 to be the backend domain. In order
>> - * to support any domain as backend we'd need to add a way to communicate
>> - * the domid of this backend, e.g. via Xenstore, via the PCI-device's
>> - * config space or DT/ACPI.
>>    */
>>   static void *xen_grant_dma_alloc(struct device *dev, size_t size,
>>   				 dma_addr_t *dma_handle, gfp_t gfp,
>> @@ -277,6 +272,16 @@ static const struct dma_map_ops xen_grant_dma_ops = {
>>   	.dma_supported = xen_grant_dma_supported,
>>   };
>>   
>> +bool xen_is_grant_dma_device(struct device *dev)
>> +{
>> +	/* XXX Handle only DT devices for now */
>> +	if (!dev->of_node)
>> +		return false;
>> +
>> +	return of_property_read_bool(dev->of_node, "xen,dev-domid");
>> +}
>> +EXPORT_SYMBOL_GPL(xen_is_grant_dma_device);
>> +
>>   void xen_grant_setup_dma_ops(struct device *dev)
>>   {
>>   	struct xen_grant_dma_data *data;
>> @@ -288,8 +293,14 @@ void xen_grant_setup_dma_ops(struct device *dev)
>>   		return;
>>   	}
>>   
>> -	/* XXX The dom0 is hardcoded as the backend domain for now */
>> -	dev_domid = 0;
>> +	/* XXX ACPI and PCI devices unsupported for now */
>> +	if (dev_is_pci(dev) || !dev->of_node)
>> +		goto err;
> I think we can remove the "dev_is_pci" check, right?

I think, yes (at least for now). I will remove the inclusion of #include 
<linux/pci.h> as well.


>
>
>> +	if (of_property_read_u32(dev->of_node, "xen,dev-domid", &dev_domid)) {
>> +		dev_err(dev, "xen,dev-domid property is not present\n");
>> +		goto err;
>> +	}
>>   
>>   	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>>   	if (!data) {
>> diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
>> index 4f9fad5..62be9dc 100644
>> --- a/include/xen/xen-ops.h
>> +++ b/include/xen/xen-ops.h
>> @@ -223,10 +223,15 @@ static inline void xen_preemptible_hcall_end(void) { }
>>   
>>   #ifdef CONFIG_XEN_GRANT_DMA_OPS
>>   void xen_grant_setup_dma_ops(struct device *dev);
>> +bool xen_is_grant_dma_device(struct device *dev);
>>   #else
>>   static inline void xen_grant_setup_dma_ops(struct device *dev)
>>   {
>>   }
>> +static inline bool xen_is_grant_dma_device(struct device *dev)
>> +{
>> +	return false;
>> +}
>>   #endif /* CONFIG_XEN_GRANT_DMA_OPS */
>>   
>>   #endif /* INCLUDE_XEN_OPS_H */
>> -- 
>> 2.7.4
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
diff mbox series

Patch

diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c
index 0e69aa8..70d5d77 100644
--- a/drivers/xen/grant-dma-ops.c
+++ b/drivers/xen/grant-dma-ops.c
@@ -66,11 +66,6 @@  static struct xen_grant_dma_data *find_xen_grant_dma_data(struct device *dev)
  * Such a DMA address is formed by using the grant reference as a frame
  * number and setting the highest address bit (this bit is for the backend
  * to be able to distinguish it from e.g. a mmio address).
- *
- * Note that for now we hard wire dom0 to be the backend domain. In order
- * to support any domain as backend we'd need to add a way to communicate
- * the domid of this backend, e.g. via Xenstore, via the PCI-device's
- * config space or DT/ACPI.
  */
 static void *xen_grant_dma_alloc(struct device *dev, size_t size,
 				 dma_addr_t *dma_handle, gfp_t gfp,
@@ -277,6 +272,16 @@  static const struct dma_map_ops xen_grant_dma_ops = {
 	.dma_supported = xen_grant_dma_supported,
 };
 
+bool xen_is_grant_dma_device(struct device *dev)
+{
+	/* XXX Handle only DT devices for now */
+	if (!dev->of_node)
+		return false;
+
+	return of_property_read_bool(dev->of_node, "xen,dev-domid");
+}
+EXPORT_SYMBOL_GPL(xen_is_grant_dma_device);
+
 void xen_grant_setup_dma_ops(struct device *dev)
 {
 	struct xen_grant_dma_data *data;
@@ -288,8 +293,14 @@  void xen_grant_setup_dma_ops(struct device *dev)
 		return;
 	}
 
-	/* XXX The dom0 is hardcoded as the backend domain for now */
-	dev_domid = 0;
+	/* XXX ACPI and PCI devices unsupported for now */
+	if (dev_is_pci(dev) || !dev->of_node)
+		goto err;
+
+	if (of_property_read_u32(dev->of_node, "xen,dev-domid", &dev_domid)) {
+		dev_err(dev, "xen,dev-domid property is not present\n");
+		goto err;
+	}
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data) {
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index 4f9fad5..62be9dc 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -223,10 +223,15 @@  static inline void xen_preemptible_hcall_end(void) { }
 
 #ifdef CONFIG_XEN_GRANT_DMA_OPS
 void xen_grant_setup_dma_ops(struct device *dev);
+bool xen_is_grant_dma_device(struct device *dev);
 #else
 static inline void xen_grant_setup_dma_ops(struct device *dev)
 {
 }
+static inline bool xen_is_grant_dma_device(struct device *dev)
+{
+	return false;
+}
 #endif /* CONFIG_XEN_GRANT_DMA_OPS */
 
 #endif /* INCLUDE_XEN_OPS_H */