[v3,08/17] intel_iommu: Set accessed and dirty bits during first stage translation

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          | 25 ++++++++++++++++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

On Wed, Sep 11, 2024 at 1:26 PM Zhenzhong Duan <zhenzhong.duan@intel.com> 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          | 25 ++++++++++++++++++++++++-
>  2 files changed, 27 insertions(+), 1 deletion(-)
>
> 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

Nit: let's use _MASK suffix to all or not.

>
>  /* 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 be73366a64..180505423d 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,
>  };
>
> @@ -1941,6 +1942,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.
> @@ -1954,7 +1969,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 ","
> @@ -1992,6 +2007,14 @@ static int vtd_iova_to_flpte(IntelIOMMUState *s, VTDContextEntry *ce,
>              return -VTD_FR_PAGING_ENTRY_RSVD;
>          }
>
> +        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)) {
>              *flptep = flpte;
>              *flpte_level = level;
> --
> 2.34.1
>

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

>-----Original Message-----
>From: Jason Wang <jasowang@redhat.com>
>Subject: Re: [PATCH v3 08/17] intel_iommu: Set accessed and dirty bits
>during first stage translation
>
>On Wed, Sep 11, 2024 at 1:26 PM Zhenzhong Duan
><zhenzhong.duan@intel.com> 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          | 25 ++++++++++++++++++++++++-
>>  2 files changed, 27 insertions(+), 1 deletion(-)
>>
>> 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
>
>Nit: let's use _MASK suffix to all or not.

Will use:

#define VTD_FL_P                    1ULL
#define VTD_FL_RW_MASK              (1ULL << 1)
#define VTD_FL_A_MASK               (1ULL << 5)
#define VTD_FL_D_MASK               (1ULL << 6)

Thanks
Zhenzhong