Re: [PATCH] iommu: Add device name to iommu map/unmap trace events

2021-04-06 Thread chenxiang (M)

Hi,


在 2021/2/12 18:50, Joerg Roedel 写道:

On Tue, Feb 09, 2021 at 06:06:20PM +0530, Sai Prakash Ranjan wrote:

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 5e7fe519430a..6064187d9bb6 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -87,6 +87,7 @@ struct iommu_domain {
void *handler_token;
struct iommu_domain_geometry geometry;
void *iova_cookie;
+   char dev_name[32];
  };

No, definitly not. A domain is a device DMA address space which can be
used by more than one device. Just look at IOMMU groups with more than
one member device, in this case just one device name would be very
misleading.


Is it possible to use group id to identify different domains?




Regards,

Joerg
___
iommu mailing list
io...@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

.






Re: [Linuxarm] Re: [PATCH v14 07/13] iommu/smmuv3: Implement cache_invalidate

2021-03-22 Thread chenxiang (M)

Hi Eric,


在 2021/3/22 17:05, Auger Eric 写道:

Hi Chenxiang,

On 3/22/21 7:40 AM, chenxiang (M) wrote:

Hi Eric,


在 2021/3/20 1:36, Auger Eric 写道:

Hi Chenxiang,

On 3/4/21 8:55 AM, chenxiang (M) wrote:

Hi Eric,


在 2021/2/24 4:56, Eric Auger 写道:

Implement domain-selective, pasid selective and page-selective
IOTLB invalidations.

Signed-off-by: Eric Auger 

---

v13 -> v14:
- Add domain invalidation
- do global inval when asid is not provided with addr
granularity

v7 -> v8:
- ASID based invalidation using iommu_inv_pasid_info
- check ARCHID/PASID flags in addr based invalidation
- use __arm_smmu_tlb_inv_context and __arm_smmu_tlb_inv_range_nosync

v6 -> v7
- check the uapi version

v3 -> v4:
- adapt to changes in the uapi
- add support for leaf parameter
- do not use arm_smmu_tlb_inv_range_nosync or arm_smmu_tlb_inv_context
anymore

v2 -> v3:
- replace __arm_smmu_tlb_sync by arm_smmu_cmdq_issue_sync

v1 -> v2:
- properly pass the asid
---
   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 74
+
   1 file changed, 74 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 4c19a1114de4..df3adc49111c 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2949,6 +2949,79 @@ static void
arm_smmu_detach_pasid_table(struct iommu_domain *domain)
   mutex_unlock(_domain->init_mutex);
   }
   +static int
+arm_smmu_cache_invalidate(struct iommu_domain *domain, struct
device *dev,
+  struct iommu_cache_invalidate_info *inv_info)
+{
+struct arm_smmu_cmdq_ent cmd = {.opcode = CMDQ_OP_TLBI_NSNH_ALL};
+struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+struct arm_smmu_device *smmu = smmu_domain->smmu;
+
+if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+return -EINVAL;
+
+if (!smmu)
+return -EINVAL;
+
+if (inv_info->version != IOMMU_CACHE_INVALIDATE_INFO_VERSION_1)
+return -EINVAL;
+
+if (inv_info->cache & IOMMU_CACHE_INV_TYPE_PASID ||
+inv_info->cache & IOMMU_CACHE_INV_TYPE_DEV_IOTLB) {
+return -ENOENT;
+}
+
+if (!(inv_info->cache & IOMMU_CACHE_INV_TYPE_IOTLB))
+return -EINVAL;
+
+/* IOTLB invalidation */
+
+switch (inv_info->granularity) {
+case IOMMU_INV_GRANU_PASID:
+{
+struct iommu_inv_pasid_info *info =
+_info->granu.pasid_info;
+
+if (info->flags & IOMMU_INV_ADDR_FLAGS_PASID)
+return -ENOENT;
+if (!(info->flags & IOMMU_INV_PASID_FLAGS_ARCHID))
+return -EINVAL;
+
+__arm_smmu_tlb_inv_context(smmu_domain, info->archid);
+return 0;
+}
+case IOMMU_INV_GRANU_ADDR:
+{
+struct iommu_inv_addr_info *info = _info->granu.addr_info;
+size_t size = info->nb_granules * info->granule_size;
+bool leaf = info->flags & IOMMU_INV_ADDR_FLAGS_LEAF;
+
+if (info->flags & IOMMU_INV_ADDR_FLAGS_PASID)
+return -ENOENT;
+
+if (!(info->flags & IOMMU_INV_ADDR_FLAGS_ARCHID))
+break;
+
+arm_smmu_tlb_inv_range_domain(info->addr, size,
+  info->granule_size, leaf,
+  info->archid, smmu_domain);

Is it possible to add a check before the function to make sure that
info->granule_size can be recognized by SMMU?
There is a scenario which will cause TLBI issue: RIL feature is enabled
on guest but is disabled on host, and then on
host it just invalidate 4K/2M/1G once a time, but from QEMU,
info->nb_granules is always 1 and info->granule_size = size,
if size is not equal to 4K or 2M or 1G (for example size = granule_size
is 5M), it will only part of the size it wants to invalidate.

Do you have any idea about this issue?

At the QEMU VFIO notifier level, when I fill the struct
iommu_cache_invalidate_info, I currently miss the granule info, hence
this weird choice of setting setting info->nb_granules is always 1 and
info->granule_size = size. I think in arm_smmu_cache_invalidate() I need
to convert this info back to a the leaf page size, in case the host does
not support RIL. Just as it is done in  __arm_smmu_tlb_inv_range if RIL
is supported.

Does it makes sense to you?


Yes, it makes sense to me.
I am glad to test them if the patchset are ready.




Thank you for spotting the issue.

Eric

I think maybe we can add a check here: if RIL is not enabled and also
size is not the granule_size (4K/2M/1G) supported by
SMMU hardware, can we just simply use the smallest granule_size
supported by hardware all the time?


+
+arm_smmu_cmdq_issue_sync(smmu);
+return 0;
+}
+case IOMMU_INV_GRANU_DOMAIN:
+break;

I check the qemu code
(https://github.com/eauger/qemu/tree/v5.2.0-2stage-rfcv8), for opcode
CMD_TLBI_NH_A

Re: [Linuxarm] Re: [PATCH v14 07/13] iommu/smmuv3: Implement cache_invalidate

2021-03-22 Thread chenxiang (M)

Hi Eric,


在 2021/3/20 1:36, Auger Eric 写道:

Hi Chenxiang,

On 3/4/21 8:55 AM, chenxiang (M) wrote:

Hi Eric,


在 2021/2/24 4:56, Eric Auger 写道:

Implement domain-selective, pasid selective and page-selective
IOTLB invalidations.

Signed-off-by: Eric Auger 

---

v13 -> v14:
- Add domain invalidation
- do global inval when asid is not provided with addr
   granularity

v7 -> v8:
- ASID based invalidation using iommu_inv_pasid_info
- check ARCHID/PASID flags in addr based invalidation
- use __arm_smmu_tlb_inv_context and __arm_smmu_tlb_inv_range_nosync

v6 -> v7
- check the uapi version

v3 -> v4:
- adapt to changes in the uapi
- add support for leaf parameter
- do not use arm_smmu_tlb_inv_range_nosync or arm_smmu_tlb_inv_context
   anymore

v2 -> v3:
- replace __arm_smmu_tlb_sync by arm_smmu_cmdq_issue_sync

v1 -> v2:
- properly pass the asid
---
  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 74 +
  1 file changed, 74 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 4c19a1114de4..df3adc49111c 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2949,6 +2949,79 @@ static void arm_smmu_detach_pasid_table(struct 
iommu_domain *domain)
mutex_unlock(_domain->init_mutex);
  }
  
+static int

+arm_smmu_cache_invalidate(struct iommu_domain *domain, struct device *dev,
+ struct iommu_cache_invalidate_info *inv_info)
+{
+   struct arm_smmu_cmdq_ent cmd = {.opcode = CMDQ_OP_TLBI_NSNH_ALL};
+   struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+   struct arm_smmu_device *smmu = smmu_domain->smmu;
+
+   if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+   return -EINVAL;
+
+   if (!smmu)
+   return -EINVAL;
+
+   if (inv_info->version != IOMMU_CACHE_INVALIDATE_INFO_VERSION_1)
+   return -EINVAL;
+
+   if (inv_info->cache & IOMMU_CACHE_INV_TYPE_PASID ||
+   inv_info->cache & IOMMU_CACHE_INV_TYPE_DEV_IOTLB) {
+   return -ENOENT;
+   }
+
+   if (!(inv_info->cache & IOMMU_CACHE_INV_TYPE_IOTLB))
+   return -EINVAL;
+
+   /* IOTLB invalidation */
+
+   switch (inv_info->granularity) {
+   case IOMMU_INV_GRANU_PASID:
+   {
+   struct iommu_inv_pasid_info *info =
+   _info->granu.pasid_info;
+
+   if (info->flags & IOMMU_INV_ADDR_FLAGS_PASID)
+   return -ENOENT;
+   if (!(info->flags & IOMMU_INV_PASID_FLAGS_ARCHID))
+   return -EINVAL;
+
+   __arm_smmu_tlb_inv_context(smmu_domain, info->archid);
+   return 0;
+   }
+   case IOMMU_INV_GRANU_ADDR:
+   {
+   struct iommu_inv_addr_info *info = _info->granu.addr_info;
+   size_t size = info->nb_granules * info->granule_size;
+   bool leaf = info->flags & IOMMU_INV_ADDR_FLAGS_LEAF;
+
+   if (info->flags & IOMMU_INV_ADDR_FLAGS_PASID)
+   return -ENOENT;
+
+   if (!(info->flags & IOMMU_INV_ADDR_FLAGS_ARCHID))
+   break;
+
+   arm_smmu_tlb_inv_range_domain(info->addr, size,
+ info->granule_size, leaf,
+ info->archid, smmu_domain);

Is it possible to add a check before the function to make sure that
info->granule_size can be recognized by SMMU?
There is a scenario which will cause TLBI issue: RIL feature is enabled
on guest but is disabled on host, and then on
host it just invalidate 4K/2M/1G once a time, but from QEMU,
info->nb_granules is always 1 and info->granule_size = size,
if size is not equal to 4K or 2M or 1G (for example size = granule_size
is 5M), it will only part of the size it wants to invalidate.


Do you have any idea about this issue?



I think maybe we can add a check here: if RIL is not enabled and also
size is not the granule_size (4K/2M/1G) supported by
SMMU hardware, can we just simply use the smallest granule_size
supported by hardware all the time?


+
+   arm_smmu_cmdq_issue_sync(smmu);
+   return 0;
+   }
+   case IOMMU_INV_GRANU_DOMAIN:
+   break;

I check the qemu code
(https://github.com/eauger/qemu/tree/v5.2.0-2stage-rfcv8), for opcode
CMD_TLBI_NH_ALL or CMD_TLBI_NSNH_ALL from guest OS
it calls smmu_inv_notifiers_all() to unamp all notifiers of all mr's,
but it seems not set event.entry.granularity which i think it should set
IOMMU_INV_GRAN_ADDR.

this is because IOMMU_INV_GRAN_ADDR = 0. But for clarity I should rather
set it explicitly ;-)


ok


BTW, for opcode CMD_TLBI_NH_ALL or CMD_TLBI_NSNH_ALL, it needs to unmap
size = 0x1 on 48bit s

Re: [PATCH 3/3] PM: runtime: Resume the device earlier in __device_release_driver()

2020-10-22 Thread chenxiang (M)

Hi Rafael,

在 2020/10/22 3:14, Rafael J. Wysocki 写道:

From: Rafael J. Wysocki 

Since the device is resumed from runtime-suspend in
__device_release_driver() anyway, it is better to do that before
looking for busy managed device links from it to consumers, because
if there are any, device_links_unbind_consumers() will be called
and it will cause the consumer devices' drivers to unbind, so the
consumer devices will be runtime-resumed.  In turn, resuming each
consumer device will cause the supplier to be resumed and when the
runtime PM references from the given consumer to it are dropped, it
may be suspended.  Then, the runtime-resume of the next consumer
will cause the supplier to resume again and so on.

Update the code accordingly.

Signed-off-by: Rafael J. Wysocki 
Fixes: 9ed9895370ae ("driver core: Functional dependencies tracking support")
Cc: All applicable  # All applicable
---
  drivers/base/dd.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-pm/drivers/base/dd.c
===
--- linux-pm.orig/drivers/base/dd.c
+++ linux-pm/drivers/base/dd.c
@@ -1117,6 +1117,8 @@ static void __device_release_driver(stru
  
  	drv = dev->driver;

if (drv) {
+   pm_runtime_get_sync(dev);
+
while (device_links_busy(dev)) {
__device_driver_unlock(dev, parent);
  
@@ -1132,8 +1134,6 @@ static void __device_release_driver(stru


pm_runtime_put_sync() is required to be called if existed from here.


return;
}
  
-		pm_runtime_get_sync(dev);

-
driver_sysfs_remove(dev);
  
  		if (dev->bus)





.






Re: [PATCH v2 07/15] scsi: hisi_sas_v3_hw: use generic power management

2020-07-20 Thread chenxiang (M)




在 2020/7/20 21:34, Vaibhav Gupta 写道:

With legacy PM, drivers themselves were responsible for managing the
device's power states and takes care of register states.

After upgrading to the generic structure, PCI core will take care of
required tasks and drivers should do only device-specific operations.

The driver was calling pci_save/restore_state(), pci_choose_state(),
pci_enable/disable_device() and pci_set_power_state() which is no more
needed.

Compile-tested only.

Signed-off-by: Vaibhav Gupta 


Reviewed-by: Xiang Chen 


---
  drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 32 +-
  1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c 
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 55e2321a65bc..824bfbe1abbb 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -3374,13 +3374,13 @@ enum {
hip08,
  };
  
-static int hisi_sas_v3_suspend(struct pci_dev *pdev, pm_message_t state)

+static int __maybe_unused hisi_sas_v3_suspend(struct device *dev_d)
  {
+   struct pci_dev *pdev = to_pci_dev(dev_d);
struct sas_ha_struct *sha = pci_get_drvdata(pdev);
struct hisi_hba *hisi_hba = sha->lldd_ha;
struct device *dev = hisi_hba->dev;
struct Scsi_Host *shost = hisi_hba->shost;
-   pci_power_t device_state;
int rc;
  
  	if (!pdev->pm_cap) {

@@ -3406,12 +3406,7 @@ static int hisi_sas_v3_suspend(struct pci_dev *pdev, 
pm_message_t state)
  
  	hisi_sas_init_mem(hisi_hba);
  
-	device_state = pci_choose_state(pdev, state);

-   dev_warn(dev, "entering operating state [D%d]\n",
-   device_state);
-   pci_save_state(pdev);
-   pci_disable_device(pdev);
-   pci_set_power_state(pdev, device_state);
+   dev_warn(dev, "entering suspend state\n");
  
  	hisi_sas_release_tasks(hisi_hba);
  
@@ -3419,8 +3414,9 @@ static int hisi_sas_v3_suspend(struct pci_dev *pdev, pm_message_t state)

return 0;
  }
  
-static int hisi_sas_v3_resume(struct pci_dev *pdev)

+static int __maybe_unused hisi_sas_v3_resume(struct device *dev_d)
  {
+   struct pci_dev *pdev = to_pci_dev(dev_d);
struct sas_ha_struct *sha = pci_get_drvdata(pdev);
struct hisi_hba *hisi_hba = sha->lldd_ha;
struct Scsi_Host *shost = hisi_hba->shost;
@@ -3430,16 +3426,8 @@ static int hisi_sas_v3_resume(struct pci_dev *pdev)
  
  	dev_warn(dev, "resuming from operating state [D%d]\n",

 device_state);
-   pci_set_power_state(pdev, PCI_D0);
-   pci_enable_wake(pdev, PCI_D0, 0);
-   pci_restore_state(pdev);
-   rc = pci_enable_device(pdev);
-   if (rc) {
-   dev_err(dev, "enable device failed during resume (%d)\n", rc);
-   return rc;
-   }
+   device_wakeup_disable(dev_d);
  
-	pci_set_master(pdev);

scsi_unblock_requests(shost);
clear_bit(HISI_SAS_REJECT_CMD_BIT, _hba->flags);
  
@@ -3447,7 +3435,6 @@ static int hisi_sas_v3_resume(struct pci_dev *pdev)

rc = hw_init_v3_hw(hisi_hba);
if (rc) {
scsi_remove_host(shost);
-   pci_disable_device(pdev);
return rc;
}
hisi_hba->hw->phys_init(hisi_hba);
@@ -3468,13 +3455,16 @@ static const struct pci_error_handlers 
hisi_sas_err_handler = {
.reset_done = hisi_sas_reset_done_v3_hw,
  };
  
+static SIMPLE_DEV_PM_OPS(hisi_sas_v3_pm_ops,

+hisi_sas_v3_suspend,
+hisi_sas_v3_resume);
+
  static struct pci_driver sas_v3_pci_driver = {
.name   = DRV_NAME,
.id_table   = sas_v3_pci_table,
.probe  = hisi_sas_v3_probe,
.remove = hisi_sas_v3_remove,
-   .suspend= hisi_sas_v3_suspend,
-   .resume = hisi_sas_v3_resume,
+   .driver.pm  = _sas_v3_pm_ops,
.err_handler= _sas_err_handler,
  };
  





Re: [PATCH v1 07/15] scsi: hisi_sas_v3_hw: use generic power management

2020-07-20 Thread chenxiang (M)

Hi Vaibhav,

在 2020/7/17 14:34, Vaibhav Gupta 写道:

With legacy PM, drivers themselves were responsible for managing the
device's power states and takes care of register states.

After upgrading to the generic structure, PCI core will take care of
required tasks and drivers should do only device-specific operations.

The driver was calling pci_save/restore_state(), pci_choose_state(),
pci_enable/disable_device() and pci_set_power_state() which is no more
needed.

Compile-tested only.

Signed-off-by: Vaibhav Gupta 


Reviewed-by: Xiang Chen 
Just a small comment, below.


---
  drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 32 --
  1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c 
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 55e2321a65bc..45605a520bc8 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -3374,13 +3374,13 @@ enum {
hip08,
  };
  
-static int hisi_sas_v3_suspend(struct pci_dev *pdev, pm_message_t state)

+static int __maybe_unused hisi_sas_v3_suspend(struct device *dev_d)
  {
+   struct pci_dev *pdev = to_pci_dev(dev_d);
struct sas_ha_struct *sha = pci_get_drvdata(pdev);
struct hisi_hba *hisi_hba = sha->lldd_ha;
struct device *dev = hisi_hba->dev;
struct Scsi_Host *shost = hisi_hba->shost;
-   pci_power_t device_state;
int rc;
  
  	if (!pdev->pm_cap) {

@@ -3406,21 +3406,15 @@ static int hisi_sas_v3_suspend(struct pci_dev *pdev, 
pm_message_t state)
  
  	hisi_sas_init_mem(hisi_hba);
  
-	device_state = pci_choose_state(pdev, state);

-   dev_warn(dev, "entering operating state [D%d]\n",
-   device_state);


Please retain above print to keep consistence with the print in function 
hisi_sas_v3_resume().



-   pci_save_state(pdev);
-   pci_disable_device(pdev);
-   pci_set_power_state(pdev, device_state);
-
hisi_sas_release_tasks(hisi_hba);
  
  	sas_suspend_ha(sha);

return 0;
  }
  
-static int hisi_sas_v3_resume(struct pci_dev *pdev)

+static int __maybe_unused hisi_sas_v3_resume(struct device *dev_d)
  {
+   struct pci_dev *pdev = to_pci_dev(dev_d);
struct sas_ha_struct *sha = pci_get_drvdata(pdev);
struct hisi_hba *hisi_hba = sha->lldd_ha;
struct Scsi_Host *shost = hisi_hba->shost;
@@ -3430,16 +3424,8 @@ static int hisi_sas_v3_resume(struct pci_dev *pdev)
  
  	dev_warn(dev, "resuming from operating state [D%d]\n",

 device_state);
-   pci_set_power_state(pdev, PCI_D0);
-   pci_enable_wake(pdev, PCI_D0, 0);
-   pci_restore_state(pdev);
-   rc = pci_enable_device(pdev);
-   if (rc) {
-   dev_err(dev, "enable device failed during resume (%d)\n", rc);
-   return rc;
-   }
+   device_wakeup_disable(dev_d);
  
-	pci_set_master(pdev);

scsi_unblock_requests(shost);
clear_bit(HISI_SAS_REJECT_CMD_BIT, _hba->flags);
  
@@ -3447,7 +3433,6 @@ static int hisi_sas_v3_resume(struct pci_dev *pdev)

rc = hw_init_v3_hw(hisi_hba);
if (rc) {
scsi_remove_host(shost);
-   pci_disable_device(pdev);
return rc;
}
hisi_hba->hw->phys_init(hisi_hba);
@@ -3468,13 +3453,16 @@ static const struct pci_error_handlers 
hisi_sas_err_handler = {
.reset_done = hisi_sas_reset_done_v3_hw,
  };
  
+static SIMPLE_DEV_PM_OPS(hisi_sas_v3_pm_ops,

+hisi_sas_v3_suspend,
+hisi_sas_v3_resume);
+
  static struct pci_driver sas_v3_pci_driver = {
.name   = DRV_NAME,
.id_table   = sas_v3_pci_table,
.probe  = hisi_sas_v3_probe,
.remove = hisi_sas_v3_remove,
-   .suspend= hisi_sas_v3_suspend,
-   .resume = hisi_sas_v3_resume,
+   .driver.pm  = _sas_v3_pm_ops,
.err_handler= _sas_err_handler,
  };
  





Re: [PATCH 1/6] scsi: hisi_sas: Add support for DIX feature for v3 hw

2019-02-11 Thread chenxiang (M)

Hi Martin,

在 2019/2/9 7:13, Martin K. Petersen 写道:

John,

Just noticed this while inspecting the resulting complete diff:


  static u32 get_prot_chk_msk_v3_hw(struct scsi_cmnd *scsi_cmnd)
  {
unsigned char prot_flags = scsi_cmnd->prot_flags;
  
-	if (prot_flags & SCSI_PROT_TRANSFER_PI) {

-   if (prot_flags & SCSI_PROT_REF_CHECK)
-   return 0xc << 16;
-   return 0xfc << 16;
-   }
-   return 0;
+   if (prot_flags & SCSI_PROT_REF_CHECK)
+   return T10_CHK_APP_TAG_MSK;

Polarity is a bit unclear here. Is this statement disabling checking of
the app tag?


Yes, disabling checking of app tag.
For here, 1'b0 presents the related byte in DIF will be checked, and 
1'b1 presents the related byte in DIF will not be checked.






+   return T10_CHK_REF_TAG_MSK | T10_CHK_APP_TAG_MSK;
  }





Re: [PATCH] block: set rq->cmd_flags with bio->opf instead of data->cmd_flags when bio is not Null

2019-01-28 Thread chenxiang (M)




在 2019/1/28 23:57, Christoph Hellwig 写道:

On Mon, Jan 28, 2019 at 03:36:58PM +, John Garry wrote:

As I understood, the problem is the scenario of calling
blk_mq_make_request()->bio_integrity_prep() where we then allocate a bio
integrity payload in calling bio_integrity_alloc().

In this case, bio_integrity_alloc() sets bio->bi_opf |= REQ_INTEGRITY, which
is no longer consistent with data.cmd_flags.

I don't see how that could happen:

static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
{
...

if (!bio_integrity_prep(bio))
return BLK_QC_T_NONE;

...

data.cmd_flags = bio->bi_opf;
 rq = blk_mq_get_request(q, bio, );


Sorry to disturb, i used kernel 5.0-rc1 which has the issue, and it is 
fixed on linux-next branch.





.






Re: [PATCH] block: set rq->cmd_flags with bio->opf instead of data->cmd_flags when bio is not Null

2019-01-24 Thread chenxiang (M)

+cc Jens + linux-block

在 2019/1/24 21:43, chenxiang 写道:

In function blk_mq_make_request(), though data->cmd_flags will be
initialized with bio->opf, later bio->opf may be set as REQ_INTEGRITY
if enabled DIX. So need to use bio->opf instead of data->cmd_flags in
function blk_mq_rq_ctx_init(), or  flags REQ_INTEGRITY will not be set
for rq->cmd_flags. It will cause dix=0 in function
sd_setup_read_write_cmnd() when enabled DIX, which will cause IO
exception when enabled DIX.

For some IOs such as internal IO from SCSI layer, the parameter bio of
function blk_mq_get_request() is Null, so need to check bio to
decise rq->cmd_flags.

Fixes: f9afca4d367b ("blk-mq: pass in request/bio flags to queue mapping")

Signed-off-by: Xiang Chen 
Reviewed-by: John Garry 
---
  block/blk-mq.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 3ba37b9..c4a1c63 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -394,7 +394,7 @@ static struct request *blk_mq_get_request(struct 
request_queue *q,
return NULL;
}
  
-	rq = blk_mq_rq_ctx_init(data, tag, data->cmd_flags);

+   rq = blk_mq_rq_ctx_init(data, tag, bio ? bio->bi_opf : data->cmd_flags);
if (!op_is_flush(data->cmd_flags)) {
rq->elv.icq = NULL;
if (e && e->type->ops.prepare_request) {





Re: linux-next: Signed-off-by missing for commit in the scsi tree

2017-12-18 Thread chenxiang (M)

在 2017/12/19 12:27, Martin K. Petersen 写道:

Stephen,


   9188808c7760 ("scsi: hisi_sas: fix SAS_QUEUE_FULL problem while running IO")

is missing a Signed-off-by from its author.

John?


Hi Stephen and Martin,
John is on vacation. I have checked it and please add a "Signed-off-by: 
Xiang Chen " or let me know if want us to 
re-send this patch again .









Re: linux-next: Signed-off-by missing for commit in the scsi tree

2017-12-18 Thread chenxiang (M)

在 2017/12/19 12:27, Martin K. Petersen 写道:

Stephen,


   9188808c7760 ("scsi: hisi_sas: fix SAS_QUEUE_FULL problem while running IO")

is missing a Signed-off-by from its author.

John?


Hi Stephen and Martin,
John is on vacation. I have checked it and please add a "Signed-off-by: 
Xiang Chen " or let me know if want us to 
re-send this patch again .