Re: [Xen-devel] [PATCH 2/2] amd-iommu: replace occurrences of u<N> with uint<N>_t...

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 26 November 2018 09:46
> 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 2/2] amd-iommu: replace occurrences of
> u<N> with uint<N>_t...
> 
> >>> On 26.11.18 at 10:05, <paul.durrant@xxxxxxxxxx> wrote:
> > @@ -127,24 +127,25 @@ static bool set_iommu_pte_present(unsigned long
> pt_mfn, unsigned long dfn,
> >                                    unsigned long next_mfn, int
> pde_level,
> >                                    bool iw, bool ir)
> >  {
> > -    u64 *table;
> > -    u32 *pde;
> > +    uint64_t *table;
> > +    uint32_t *pde;
> >      bool need_flush;
> >
> >      table = map_domain_page(_mfn(pt_mfn));
> >
> > -    pde = (u32*)(table + pfn_to_pde_idx(dfn, pde_level));
> > +    pde = (uint32_t*)(table + pfn_to_pde_idx(dfn, pde_level));
> 
> Please add the missing blank here at the same time.

Ok.

> 
> > @@ -229,11 +230,12 @@ void __init amd_iommu_set_intremap_table(
> >      dte[4] = entry;
> >  }
> >
> > -void __init iommu_dte_add_device_entry(u32 *dte, struct ivrs_mappings
> > *ivrs_dev)
> > +void __init iommu_dte_add_device_entry(uint32_t *dte,
> > +                                       struct ivrs_mappings *ivrs_dev)
> >  {
> > -    u32 entry;
> > -    u8 sys_mgt, dev_ex, flags;
> > -    u8 mask = ~(0x7 << 3);
> > +    uint32_t entry;
> > +    uint8_t sys_mgt, dev_ex, flags;
> > +    uint8_t mask = ~(0x7 << 3);
> 
> I question the use of 8-bit fixed width types here.

I was trying to keep this as mechanical as possible, and certainly 
non-functional so I don't really want to go messing with the choice of types in 
this patch.

> 
> > @@ -486,8 +488,8 @@ static int iommu_pde_from_dfn(struct domain *d,
> unsigned long dfn,
> >          next_table_mfn = amd_iommu_get_address_from_pte(pde) >>
> PAGE_SHIFT;
> >
> >          /* Split super page frame into smaller pieces.*/
> > -        if ( iommu_is_pte_present((u32*)pde) &&
> > -             (iommu_next_level((u32*)pde) == 0) &&
> > +        if ( iommu_is_pte_present((uint32_t*)pde) &&
> > +             (iommu_next_level((uint32_t*)pde) == 0) &&
> 
> Blanks to be added again. More further down.
> 

Ok.

> > @@ -805,7 +808,7 @@ int amd_iommu_unmap_page(struct domain *d, dfn_t
> dfn)
> >  }
> >
> >  int amd_iommu_reserve_domain_unity_map(struct domain *domain,
> > -                                       u64 phys_addr,
> > +                                       uint64_t phys_addr,
> 
> Transformations like this are also a little odd to see - why not switch
> to paddr_t at this occasion?
> 

Again, trying to keep the changes in this patch basically mechanical, but 
moving to an abstract type here seems like a reasonably obvious tweak.

  Paul

> Jan
> 


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