Re: [Xen-devel] [PATCH] amd-iommu: use correct constants in amd_iommu_get_next_table_from_pte()...

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 26 September 2018 14:32
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: Brian Woods <brian.woods@xxxxxxx>; Suravee Suthikulpanit
> <suravee.suthikulpanit@xxxxxxx>; xen-devel <xen-
> devel@xxxxxxxxxxxxxxxxxxxx>
> Subject: Re: [Xen-devel] [PATCH] amd-iommu: use correct constants in
> amd_iommu_get_next_table_from_pte()...
> 
> >>> On 26.09.18 at 14:56, <paul.durrant@xxxxxxxxxx> wrote:
> > ...and change the name to amd_iommu_get_address_from_pte() since the
> > address read is not necessarily a 'next level' page table.
> 
> There was no "level" in the original name. Which isn't meant to be an
> objection to the rename.

Ok. I'll re-word it a bit.

> 
> > --- a/xen/drivers/passthrough/amd/iommu_map.c
> > +++ b/xen/drivers/passthrough/amd/iommu_map.c
> > @@ -285,19 +285,18 @@ void iommu_dte_set_guest_cr3(u32 *dte, u16 dom_id,
> u64 gcr3,
> >      dte[1] = entry;
> >  }
> >
> > -u64 amd_iommu_get_next_table_from_pte(u32 *entry)
> > +uint64_t amd_iommu_get_address_from_pte(void *pte)
> >  {
> > -    u64 addr_lo, addr_hi, ptr;
> > +    uint32_t *entry = pte;
> > +    uint64_t addr_lo, addr_hi, ptr;
> >
> > -    addr_lo = get_field_from_reg_u32(
> > -        entry[0],
> > -        IOMMU_DEV_TABLE_PAGE_TABLE_PTR_LOW_MASK,
> > -        IOMMU_DEV_TABLE_PAGE_TABLE_PTR_LOW_SHIFT);
> > +    addr_lo = get_field_from_reg_u32(entry[0],
> > +                                     IOMMU_PDE_ADDR_LOW_MASK,
> > +                                     IOMMU_PDE_ADDR_LOW_SHIFT);
> >
> > -    addr_hi = get_field_from_reg_u32(
> > -        entry[1],
> > -        IOMMU_DEV_TABLE_PAGE_TABLE_PTR_HIGH_MASK,
> > -        IOMMU_DEV_TABLE_PAGE_TABLE_PTR_HIGH_SHIFT);
> > +    addr_hi = get_field_from_reg_u32(entry[1],
> > +                                     IOMMU_PDE_ADDR_HIGH_MASK,
> > +                                     IOMMU_PDE_ADDR_HIGH_SHIFT);
> 
> With the function parameter named pte, perhaps better to also use
> IOMMU_PTE_ADDR_* (which is also more generic imo, as it covers
> both leaf and non-leaf entries).
> 

Yes, agreed.

  Paul

> Jan
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel