From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
hw/i386/intel_iommu_internal.h | 3 +++
hw/i386/intel_iommu.c | 24 ++++++++++++++++++++++++
2 files changed, 27 insertions(+)
diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index 668583aeca..7786ef7624 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -324,6 +324,7 @@ typedef enum VTDFaultReason {
/* Output address in the interrupt address range for scalable mode */
VTD_FR_SM_INTERRUPT_ADDR = 0x87,
+ VTD_FR_FS_BIT_UPDATE_FAILED = 0x91, /* SFS.10 */
VTD_FR_MAX, /* Guard */
} VTDFaultReason;
@@ -549,6 +550,8 @@ typedef struct VTDRootEntry VTDRootEntry;
/* Masks for First Level Paging Entry */
#define VTD_FL_P 1ULL
#define VTD_FL_RW_MASK (1ULL << 1)
+#define VTD_FL_A 0x20
+#define VTD_FL_D 0x40
/* Second Level Page Translation Pointer*/
#define VTD_SM_PASID_ENTRY_SLPTPTR (~0xfffULL)
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 6121cca4cd..3c2ceed284 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -1822,6 +1822,7 @@ static const bool vtd_qualified_faults[] = {
[VTD_FR_PASID_TABLE_ENTRY_INV] = true,
[VTD_FR_SM_INTERRUPT_ADDR] = true,
[VTD_FR_FS_NON_CANONICAL] = true,
+ [VTD_FR_FS_BIT_UPDATE_FAILED] = true,
[VTD_FR_MAX] = false,
};
@@ -1939,6 +1940,20 @@ static bool vtd_iova_fl_check_canonical(IntelIOMMUState *s, uint64_t iova,
);
}
+static MemTxResult vtd_set_flag_in_pte(dma_addr_t base_addr, uint32_t index,
+ uint64_t pte, uint64_t flag)
+{
+ if (pte & flag) {
+ return MEMTX_OK;
+ }
+ pte |= flag;
+ pte = cpu_to_le64(pte);
+ return dma_memory_write(&address_space_memory,
+ base_addr + index * sizeof(pte),
+ &pte, sizeof(pte),
+ MEMTXATTRS_UNSPECIFIED);
+}
+
/*
* Given the @iova, get relevant @flptep. @flpte_level will be the last level
* of the translation, can be used for deciding the size of large page.
@@ -1990,7 +2005,16 @@ static int vtd_iova_to_flpte(IntelIOMMUState *s, VTDContextEntry *ce,
return -VTD_FR_PAGING_ENTRY_RSVD;
}
+ if (vtd_set_flag_in_pte(addr, offset, flpte, VTD_FL_A) != MEMTX_OK) {
+ return -VTD_FR_FS_BIT_UPDATE_FAILED;
+ }
+
if (vtd_is_last_pte(flpte, level)) {
+ if (is_write &&
+ (vtd_set_flag_in_pte(addr, offset, flpte, VTD_FL_D) !=
+ MEMTX_OK)) {
+ return -VTD_FR_FS_BIT_UPDATE_FAILED;
+ }
*flptep = flpte;
*flpte_level = level;
return 0;
--
2.34.1
On 2024/8/5 14:27, Zhenzhong Duan wrote:
> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>
> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
> hw/i386/intel_iommu_internal.h | 3 +++
> hw/i386/intel_iommu.c | 24 ++++++++++++++++++++++++
> 2 files changed, 27 insertions(+)
>
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> index 668583aeca..7786ef7624 100644
> --- a/hw/i386/intel_iommu_internal.h
> +++ b/hw/i386/intel_iommu_internal.h
> @@ -324,6 +324,7 @@ typedef enum VTDFaultReason {
>
> /* Output address in the interrupt address range for scalable mode */
> VTD_FR_SM_INTERRUPT_ADDR = 0x87,
> + VTD_FR_FS_BIT_UPDATE_FAILED = 0x91, /* SFS.10 */
> VTD_FR_MAX, /* Guard */
> } VTDFaultReason;
>
> @@ -549,6 +550,8 @@ typedef struct VTDRootEntry VTDRootEntry;
> /* Masks for First Level Paging Entry */
> #define VTD_FL_P 1ULL
> #define VTD_FL_RW_MASK (1ULL << 1)
> +#define VTD_FL_A 0x20
> +#define VTD_FL_D 0x40
>
> /* Second Level Page Translation Pointer*/
> #define VTD_SM_PASID_ENTRY_SLPTPTR (~0xfffULL)
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 6121cca4cd..3c2ceed284 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -1822,6 +1822,7 @@ static const bool vtd_qualified_faults[] = {
> [VTD_FR_PASID_TABLE_ENTRY_INV] = true,
> [VTD_FR_SM_INTERRUPT_ADDR] = true,
> [VTD_FR_FS_NON_CANONICAL] = true,
> + [VTD_FR_FS_BIT_UPDATE_FAILED] = true,
> [VTD_FR_MAX] = false,
> };
>
> @@ -1939,6 +1940,20 @@ static bool vtd_iova_fl_check_canonical(IntelIOMMUState *s, uint64_t iova,
> );
> }
>
> +static MemTxResult vtd_set_flag_in_pte(dma_addr_t base_addr, uint32_t index,
> + uint64_t pte, uint64_t flag)
> +{
> + if (pte & flag) {
> + return MEMTX_OK;
> + }
> + pte |= flag;
> + pte = cpu_to_le64(pte);
> + return dma_memory_write(&address_space_memory,
> + base_addr + index * sizeof(pte),
> + &pte, sizeof(pte),
> + MEMTXATTRS_UNSPECIFIED);
Can we ensure this write is atomic? A/D bit setting should be atomic from
guest p.o.v.
> +}
> +
> /*
> * Given the @iova, get relevant @flptep. @flpte_level will be the last level
> * of the translation, can be used for deciding the size of large page.
> @@ -1990,7 +2005,16 @@ static int vtd_iova_to_flpte(IntelIOMMUState *s, VTDContextEntry *ce,
> return -VTD_FR_PAGING_ENTRY_RSVD;
> }
>
> + if (vtd_set_flag_in_pte(addr, offset, flpte, VTD_FL_A) != MEMTX_OK) {
> + return -VTD_FR_FS_BIT_UPDATE_FAILED;
> + }
> +
> if (vtd_is_last_pte(flpte, level)) {
> + if (is_write &&
> + (vtd_set_flag_in_pte(addr, offset, flpte, VTD_FL_D) !=
> + MEMTX_OK)) {
> + return -VTD_FR_FS_BIT_UPDATE_FAILED;
> + }
> *flptep = flpte;
> *flpte_level = level;
> return 0;
--
Regards,
Yi Liu
On 14/08/2024 13:45, Yi Liu wrote:
> Caution: External email. Do not open attachments or click links,
> unless this email comes from a known sender and you know the content
> is safe.
>
>
> On 2024/8/5 14:27, Zhenzhong Duan wrote:
>> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>>
>> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>> hw/i386/intel_iommu_internal.h | 3 +++
>> hw/i386/intel_iommu.c | 24 ++++++++++++++++++++++++
>> 2 files changed, 27 insertions(+)
>>
>> diff --git a/hw/i386/intel_iommu_internal.h
>> b/hw/i386/intel_iommu_internal.h
>> index 668583aeca..7786ef7624 100644
>> --- a/hw/i386/intel_iommu_internal.h
>> +++ b/hw/i386/intel_iommu_internal.h
>> @@ -324,6 +324,7 @@ typedef enum VTDFaultReason {
>>
>> /* Output address in the interrupt address range for scalable
>> mode */
>> VTD_FR_SM_INTERRUPT_ADDR = 0x87,
>> + VTD_FR_FS_BIT_UPDATE_FAILED = 0x91, /* SFS.10 */
>> VTD_FR_MAX, /* Guard */
>> } VTDFaultReason;
>>
>> @@ -549,6 +550,8 @@ typedef struct VTDRootEntry VTDRootEntry;
>> /* Masks for First Level Paging Entry */
>> #define VTD_FL_P 1ULL
>> #define VTD_FL_RW_MASK (1ULL << 1)
>> +#define VTD_FL_A 0x20
>> +#define VTD_FL_D 0x40
>>
>> /* Second Level Page Translation Pointer*/
>> #define VTD_SM_PASID_ENTRY_SLPTPTR (~0xfffULL)
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 6121cca4cd..3c2ceed284 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -1822,6 +1822,7 @@ static const bool vtd_qualified_faults[] = {
>> [VTD_FR_PASID_TABLE_ENTRY_INV] = true,
>> [VTD_FR_SM_INTERRUPT_ADDR] = true,
>> [VTD_FR_FS_NON_CANONICAL] = true,
>> + [VTD_FR_FS_BIT_UPDATE_FAILED] = true,
>> [VTD_FR_MAX] = false,
>> };
>>
>> @@ -1939,6 +1940,20 @@ static bool
>> vtd_iova_fl_check_canonical(IntelIOMMUState *s, uint64_t iova,
>> );
>> }
>>
>> +static MemTxResult vtd_set_flag_in_pte(dma_addr_t base_addr,
>> uint32_t index,
>> + uint64_t pte, uint64_t flag)
>> +{
>> + if (pte & flag) {
>> + return MEMTX_OK;
>> + }
>> + pte |= flag;
>> + pte = cpu_to_le64(pte);
>> + return dma_memory_write(&address_space_memory,
>> + base_addr + index * sizeof(pte),
>> + &pte, sizeof(pte),
>> + MEMTXATTRS_UNSPECIFIED);
>
> Can we ensure this write is atomic? A/D bit setting should be atomic from
> guest p.o.v.
As we only set one bit at a time, I don't think we can face atomicity issues
>
>> +}
>> +
>> /*
>> * Given the @iova, get relevant @flptep. @flpte_level will be the
>> last level
>> * of the translation, can be used for deciding the size of large
>> page.
>> @@ -1990,7 +2005,16 @@ static int vtd_iova_to_flpte(IntelIOMMUState
>> *s, VTDContextEntry *ce,
>> return -VTD_FR_PAGING_ENTRY_RSVD;
>> }
>>
>> + if (vtd_set_flag_in_pte(addr, offset, flpte, VTD_FL_A) !=
>> MEMTX_OK) {
>> + return -VTD_FR_FS_BIT_UPDATE_FAILED;
>> + }
>> +
>> if (vtd_is_last_pte(flpte, level)) {
>> + if (is_write &&
>> + (vtd_set_flag_in_pte(addr, offset, flpte, VTD_FL_D) !=
>> + MEMTX_OK)) {
>> + return -VTD_FR_FS_BIT_UPDATE_FAILED;
>> + }
>> *flptep = flpte;
>> *flpte_level = level;
>> return 0;
>
> --
> Regards,
> Yi Liu
>-----Original Message-----
>From: Liu, Yi L <yi.l.liu@intel.com>
>Subject: Re: [PATCH v2 08/17] intel_iommu: Set accessed and dirty bits
>during first stage translation
>
>On 2024/8/5 14:27, Zhenzhong Duan wrote:
>> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>>
>> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>> hw/i386/intel_iommu_internal.h | 3 +++
>> hw/i386/intel_iommu.c | 24 ++++++++++++++++++++++++
>> 2 files changed, 27 insertions(+)
>>
>> diff --git a/hw/i386/intel_iommu_internal.h
>b/hw/i386/intel_iommu_internal.h
>> index 668583aeca..7786ef7624 100644
>> --- a/hw/i386/intel_iommu_internal.h
>> +++ b/hw/i386/intel_iommu_internal.h
>> @@ -324,6 +324,7 @@ typedef enum VTDFaultReason {
>>
>> /* Output address in the interrupt address range for scalable mode */
>> VTD_FR_SM_INTERRUPT_ADDR = 0x87,
>> + VTD_FR_FS_BIT_UPDATE_FAILED = 0x91, /* SFS.10 */
>> VTD_FR_MAX, /* Guard */
>> } VTDFaultReason;
>>
>> @@ -549,6 +550,8 @@ typedef struct VTDRootEntry VTDRootEntry;
>> /* Masks for First Level Paging Entry */
>> #define VTD_FL_P 1ULL
>> #define VTD_FL_RW_MASK (1ULL << 1)
>> +#define VTD_FL_A 0x20
>> +#define VTD_FL_D 0x40
>>
>> /* Second Level Page Translation Pointer*/
>> #define VTD_SM_PASID_ENTRY_SLPTPTR (~0xfffULL)
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 6121cca4cd..3c2ceed284 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -1822,6 +1822,7 @@ static const bool vtd_qualified_faults[] = {
>> [VTD_FR_PASID_TABLE_ENTRY_INV] = true,
>> [VTD_FR_SM_INTERRUPT_ADDR] = true,
>> [VTD_FR_FS_NON_CANONICAL] = true,
>> + [VTD_FR_FS_BIT_UPDATE_FAILED] = true,
>> [VTD_FR_MAX] = false,
>> };
>>
>> @@ -1939,6 +1940,20 @@ static bool
>vtd_iova_fl_check_canonical(IntelIOMMUState *s, uint64_t iova,
>> );
>> }
>>
>> +static MemTxResult vtd_set_flag_in_pte(dma_addr_t base_addr,
>uint32_t index,
>> + uint64_t pte, uint64_t flag)
>> +{
>> + if (pte & flag) {
>> + return MEMTX_OK;
>> + }
>> + pte |= flag;
>> + pte = cpu_to_le64(pte);
>> + return dma_memory_write(&address_space_memory,
>> + base_addr + index * sizeof(pte),
>> + &pte, sizeof(pte),
>> + MEMTXATTRS_UNSPECIFIED);
>
>Can we ensure this write is atomic? A/D bit setting should be atomic from
>guest p.o.v.
Yes, what about below:
@@ -2096,7 +2096,7 @@ static int vtd_iova_to_flpte(IntelIOMMUState *s, VTDContextEntry *ce,
dma_addr_t addr = vtd_get_iova_pgtbl_base(s, ce, pasid);
uint32_t level = vtd_get_iova_level(s, ce, pasid);
uint32_t offset;
- uint64_t flpte;
+ uint64_t flpte, flag_ad = VTD_FL_A;
if (!vtd_iova_fl_check_canonical(s, iova, ce, pasid)) {
error_report_once("%s: detected non canonical IOVA (iova=0x%" PRIx64 ","
@@ -2134,16 +2134,15 @@ static int vtd_iova_to_flpte(IntelIOMMUState *s, VTDContextEntry *ce,
return -VTD_FR_PAGING_ENTRY_RSVD;
}
- if (vtd_set_flag_in_pte(addr, offset, flpte, VTD_FL_A) != MEMTX_OK) {
+ if (vtd_is_last_pte(flpte, level) && is_write) {
+ flag_ad |= VTD_FL_D;
+ }
+
+ if (vtd_set_flag_in_pte(addr, offset, flpte, flag_ad) != MEMTX_OK) {
return -VTD_FR_FS_BIT_UPDATE_FAILED;
}
if (vtd_is_last_pte(flpte, level)) {
- if (is_write &&
- (vtd_set_flag_in_pte(addr, offset, flpte, VTD_FL_D) !=
- MEMTX_OK)) {
- return -VTD_FR_FS_BIT_UPDATE_FAILED;
- }
*flptep = flpte;
*flpte_level = level;
return 0;
Thanks
Zhenzhong
On 16/08/2024 04:37, Duan, Zhenzhong wrote:
> Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.
>
>
>> -----Original Message-----
>> From: Liu, Yi L <yi.l.liu@intel.com>
>> Subject: Re: [PATCH v2 08/17] intel_iommu: Set accessed and dirty bits
>> during first stage translation
>>
>> On 2024/8/5 14:27, Zhenzhong Duan wrote:
>>> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>>>
>>> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>> ---
>>> hw/i386/intel_iommu_internal.h | 3 +++
>>> hw/i386/intel_iommu.c | 24 ++++++++++++++++++++++++
>>> 2 files changed, 27 insertions(+)
>>>
>>> diff --git a/hw/i386/intel_iommu_internal.h
>> b/hw/i386/intel_iommu_internal.h
>>> index 668583aeca..7786ef7624 100644
>>> --- a/hw/i386/intel_iommu_internal.h
>>> +++ b/hw/i386/intel_iommu_internal.h
>>> @@ -324,6 +324,7 @@ typedef enum VTDFaultReason {
>>>
>>> /* Output address in the interrupt address range for scalable mode */
>>> VTD_FR_SM_INTERRUPT_ADDR = 0x87,
>>> + VTD_FR_FS_BIT_UPDATE_FAILED = 0x91, /* SFS.10 */
>>> VTD_FR_MAX, /* Guard */
>>> } VTDFaultReason;
>>>
>>> @@ -549,6 +550,8 @@ typedef struct VTDRootEntry VTDRootEntry;
>>> /* Masks for First Level Paging Entry */
>>> #define VTD_FL_P 1ULL
>>> #define VTD_FL_RW_MASK (1ULL << 1)
>>> +#define VTD_FL_A 0x20
>>> +#define VTD_FL_D 0x40
>>>
>>> /* Second Level Page Translation Pointer*/
>>> #define VTD_SM_PASID_ENTRY_SLPTPTR (~0xfffULL)
>>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>>> index 6121cca4cd..3c2ceed284 100644
>>> --- a/hw/i386/intel_iommu.c
>>> +++ b/hw/i386/intel_iommu.c
>>> @@ -1822,6 +1822,7 @@ static const bool vtd_qualified_faults[] = {
>>> [VTD_FR_PASID_TABLE_ENTRY_INV] = true,
>>> [VTD_FR_SM_INTERRUPT_ADDR] = true,
>>> [VTD_FR_FS_NON_CANONICAL] = true,
>>> + [VTD_FR_FS_BIT_UPDATE_FAILED] = true,
>>> [VTD_FR_MAX] = false,
>>> };
>>>
>>> @@ -1939,6 +1940,20 @@ static bool
>> vtd_iova_fl_check_canonical(IntelIOMMUState *s, uint64_t iova,
>>> );
>>> }
>>>
>>> +static MemTxResult vtd_set_flag_in_pte(dma_addr_t base_addr,
>> uint32_t index,
>>> + uint64_t pte, uint64_t flag)
>>> +{
>>> + if (pte & flag) {
>>> + return MEMTX_OK;
>>> + }
>>> + pte |= flag;
>>> + pte = cpu_to_le64(pte);
>>> + return dma_memory_write(&address_space_memory,
>>> + base_addr + index * sizeof(pte),
>>> + &pte, sizeof(pte),
>>> + MEMTXATTRS_UNSPECIFIED);
>> Can we ensure this write is atomic? A/D bit setting should be atomic from
>> guest p.o.v.
> Yes, what about below:
>
> @@ -2096,7 +2096,7 @@ static int vtd_iova_to_flpte(IntelIOMMUState *s, VTDContextEntry *ce,
> dma_addr_t addr = vtd_get_iova_pgtbl_base(s, ce, pasid);
> uint32_t level = vtd_get_iova_level(s, ce, pasid);
> uint32_t offset;
> - uint64_t flpte;
> + uint64_t flpte, flag_ad = VTD_FL_A;
>
> if (!vtd_iova_fl_check_canonical(s, iova, ce, pasid)) {
> error_report_once("%s: detected non canonical IOVA (iova=0x%" PRIx64 ","
> @@ -2134,16 +2134,15 @@ static int vtd_iova_to_flpte(IntelIOMMUState *s, VTDContextEntry *ce,
> return -VTD_FR_PAGING_ENTRY_RSVD;
> }
>
> - if (vtd_set_flag_in_pte(addr, offset, flpte, VTD_FL_A) != MEMTX_OK) {
> + if (vtd_is_last_pte(flpte, level) && is_write) {
> + flag_ad |= VTD_FL_D;
> + }
> +
> + if (vtd_set_flag_in_pte(addr, offset, flpte, flag_ad) != MEMTX_OK) {
> return -VTD_FR_FS_BIT_UPDATE_FAILED;
> }
>
> if (vtd_is_last_pte(flpte, level)) {
> - if (is_write &&
> - (vtd_set_flag_in_pte(addr, offset, flpte, VTD_FL_D) !=
> - MEMTX_OK)) {
> - return -VTD_FR_FS_BIT_UPDATE_FAILED;
> - }
> *flptep = flpte;
> *flpte_level = level;
> return 0;
lgtm
Thanks
>cmd
>
> Thanks
> Zhenzhong
>
© 2016 - 2024 Red Hat, Inc.