| Message ID | 20230530175937.24202-11-joao.m.martins@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfio: VFIO migration support with vIOMMU | expand |
On 30/5/23 19:59, Joao Martins wrote: > From: Avihai Horon <avihaih@nvidia.com> > > Implement get_attr() method and use the address width property to report > the IOMMU_ATTR_MAX_IOVA attribute. > > Signed-off-by: Avihai Horon <avihaih@nvidia.com> > Signed-off-by: Joao Martins <joao.m.martins@oracle.com> > --- > hw/i386/intel_iommu.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > index 1906f3a67960..829dd6eadc6c 100644 > --- a/hw/i386/intel_iommu.c > +++ b/hw/i386/intel_iommu.c > @@ -3876,6 +3876,13 @@ static int vtd_iommu_get_attr(IOMMUMemoryRegion *iommu_mr, > *enabled = s->dma_translation; > break; > } > + case IOMMU_ATTR_MAX_IOVA: > + { > + hwaddr *max_iova = data; Shouldn't we cast to uintptr_t to be safe? > + *max_iova = (1ULL << s->aw_bits) - 1; Alternatively: *max_iova = MAKE_64BIT_MASK(0, s->aw_bits); > + break; > + } > default: > ret = -EINVAL; > break;
On 30/05/2023 22:45, Philippe Mathieu-Daudé wrote: > On 30/5/23 19:59, Joao Martins wrote: >> From: Avihai Horon <avihaih@nvidia.com> >> >> Implement get_attr() method and use the address width property to report >> the IOMMU_ATTR_MAX_IOVA attribute. >> >> Signed-off-by: Avihai Horon <avihaih@nvidia.com> >> Signed-off-by: Joao Martins <joao.m.martins@oracle.com> >> --- >> hw/i386/intel_iommu.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c >> index 1906f3a67960..829dd6eadc6c 100644 >> --- a/hw/i386/intel_iommu.c >> +++ b/hw/i386/intel_iommu.c >> @@ -3876,6 +3876,13 @@ static int vtd_iommu_get_attr(IOMMUMemoryRegion *iommu_mr, >> *enabled = s->dma_translation; >> break; >> } >> + case IOMMU_ATTR_MAX_IOVA: >> + { >> + hwaddr *max_iova = data; > > Shouldn't we cast to uintptr_t to be safe? > Perhaps you mean something like this: diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 829dd6eadc6c..479307f1228f 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -3878,7 +3878,7 @@ static int vtd_iommu_get_attr(IOMMUMemoryRegion *iommu_mr, } case IOMMU_ATTR_MAX_IOVA: { - hwaddr *max_iova = data; + hwaddr *max_iova = (hwaddr *)(uintptr_t) data; *max_iova = (1ULL << s->aw_bits) - 1; break; I guess the thinking is to prevent 32-bit failures. >> + *max_iova = (1ULL << s->aw_bits) - 1; > > Alternatively: > > *max_iova = MAKE_64BIT_MASK(0, s->aw_bits); > I'll switch to your suggestion. Wasn't aware of this macro :) >> + break; >> + } >> default: >> ret = -EINVAL; >> break; >
On 31/5/23 11:54, Joao Martins wrote: > > > On 30/05/2023 22:45, Philippe Mathieu-Daudé wrote: >> On 30/5/23 19:59, Joao Martins wrote: >>> From: Avihai Horon <avihaih@nvidia.com> >>> >>> Implement get_attr() method and use the address width property to report >>> the IOMMU_ATTR_MAX_IOVA attribute. >>> >>> Signed-off-by: Avihai Horon <avihaih@nvidia.com> >>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com> >>> --- >>> hw/i386/intel_iommu.c | 7 +++++++ >>> 1 file changed, 7 insertions(+) >>> + case IOMMU_ATTR_MAX_IOVA: >>> + { >>> + hwaddr *max_iova = data; >> >> Shouldn't we cast to uintptr_t to be safe? >> > Perhaps you mean something like this: > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > index 829dd6eadc6c..479307f1228f 100644 > --- a/hw/i386/intel_iommu.c > +++ b/hw/i386/intel_iommu.c > @@ -3878,7 +3878,7 @@ static int vtd_iommu_get_attr(IOMMUMemoryRegion *iommu_mr, > } > case IOMMU_ATTR_MAX_IOVA: > { > - hwaddr *max_iova = data; > + hwaddr *max_iova = (hwaddr *)(uintptr_t) data; > > *max_iova = (1ULL << s->aw_bits) - 1; > break; > > I guess the thinking is to prevent 32-bit failures. Exactly. >>> + *max_iova = (1ULL << s->aw_bits) - 1; >> >> Alternatively: >> >> *max_iova = MAKE_64BIT_MASK(0, s->aw_bits); >> > > I'll switch to your suggestion. Wasn't aware of this macro :) Thanks, it is a no-brainer when reviewing.
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 1906f3a67960..829dd6eadc6c 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -3876,6 +3876,13 @@ static int vtd_iommu_get_attr(IOMMUMemoryRegion *iommu_mr, *enabled = s->dma_translation; break; } + case IOMMU_ATTR_MAX_IOVA: + { + hwaddr *max_iova = data; + + *max_iova = (1ULL << s->aw_bits) - 1; + break; + } default: ret = -EINVAL; break;