| Message ID | 20230923012511.10379-19-joao.m.martins@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | IOMMUFD Dirty Tracking | expand |
On 9/23/2023 8:25 AM, Joao Martins wrote: > Print the feature, much like other kernel-supported features. > > One can still probe its actual hw support via sysfs, regardless > of what the kernel does. > > Signed-off-by: Joao Martins <joao.m.martins@oracle.com> > --- > drivers/iommu/amd/init.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c > index 45efb7e5d725..b091a3d10819 100644 > --- a/drivers/iommu/amd/init.c > +++ b/drivers/iommu/amd/init.c > @@ -2208,6 +2208,10 @@ static void print_iommu_info(void) > > if (iommu->features & FEATURE_GAM_VAPIC) > pr_cont(" GA_vAPIC"); > + if (iommu->features & FEATURE_HASUP) > + pr_cont(" HASup"); > + if (iommu->features & FEATURE_HDSUP) > + pr_cont(" HDSup"); > > if (iommu->features & FEATURE_SNP) > pr_cont(" SNP"); Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> Thanks, Suravee
On 17/10/2023 04:48, Suthikulpanit, Suravee wrote: > On 9/23/2023 8:25 AM, Joao Martins wrote: >> Print the feature, much like other kernel-supported features. >> >> One can still probe its actual hw support via sysfs, regardless >> of what the kernel does. >> >> Signed-off-by: Joao Martins <joao.m.martins@oracle.com> >> --- >> drivers/iommu/amd/init.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c >> index 45efb7e5d725..b091a3d10819 100644 >> --- a/drivers/iommu/amd/init.c >> +++ b/drivers/iommu/amd/init.c >> @@ -2208,6 +2208,10 @@ static void print_iommu_info(void) >> if (iommu->features & FEATURE_GAM_VAPIC) >> pr_cont(" GA_vAPIC"); >> + if (iommu->features & FEATURE_HASUP) >> + pr_cont(" HASup"); >> + if (iommu->features & FEATURE_HDSUP) >> + pr_cont(" HDSup"); >> if (iommu->features & FEATURE_SNP) >> pr_cont(" SNP"); > > Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > Thanks!
Joao, On 9/23/2023 6:55 AM, Joao Martins wrote: > Print the feature, much like other kernel-supported features. > > One can still probe its actual hw support via sysfs, regardless > of what the kernel does. > > Signed-off-by: Joao Martins <joao.m.martins@oracle.com> > --- > drivers/iommu/amd/init.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c > index 45efb7e5d725..b091a3d10819 100644 > --- a/drivers/iommu/amd/init.c > +++ b/drivers/iommu/amd/init.c > @@ -2208,6 +2208,10 @@ static void print_iommu_info(void) > > if (iommu->features & FEATURE_GAM_VAPIC) > pr_cont(" GA_vAPIC"); > + if (iommu->features & FEATURE_HASUP) > + pr_cont(" HASup"); > + if (iommu->features & FEATURE_HDSUP) > + pr_cont(" HDSup"); Note that this has a conflict with iommu/next branch. But it should be fairly straight to fix it. Otherwise patch looks good to me. Reviewed-by: Vasant Hegde <vasant.hegde@amd.com> -Vasant
On 18/10/2023 09:32, Vasant Hegde wrote: > Joao, > > On 9/23/2023 6:55 AM, Joao Martins wrote: >> Print the feature, much like other kernel-supported features. >> >> One can still probe its actual hw support via sysfs, regardless >> of what the kernel does. >> >> Signed-off-by: Joao Martins <joao.m.martins@oracle.com> >> --- >> drivers/iommu/amd/init.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c >> index 45efb7e5d725..b091a3d10819 100644 >> --- a/drivers/iommu/amd/init.c >> +++ b/drivers/iommu/amd/init.c >> @@ -2208,6 +2208,10 @@ static void print_iommu_info(void) >> >> if (iommu->features & FEATURE_GAM_VAPIC) >> pr_cont(" GA_vAPIC"); >> + if (iommu->features & FEATURE_HASUP) >> + pr_cont(" HASup"); >> + if (iommu->features & FEATURE_HDSUP) >> + pr_cont(" HDSup"); > > Note that this has a conflict with iommu/next branch. But it should be fairly > straight to fix it. Otherwise patch looks good to me. > I guess it's this patch, thanks for reminding: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/commit/drivers/iommu/amd/init.c?h=next&id=7b7563a93437ef945c829538da28f0095f1603ec But then it's the same problem as the previous patch. The loop above is enterily reworked, so the code above won't work, and the "iommu->features &" conditionals needs to be replaced with a check_feature(FEATURE_HDSUP) and check_feature(FEATURE_HASUP). And depending on the order of pull requests this is problematic. The previous patch can get away with direct usage of amd_iommu_efr, but this one sadly no. I can skip this patch in particular for v4 and re-submit after -rc1 when everything is aligned. It is only for user experience about console printing two strings. Real feature probe is not affected users still have the old sysfs interface, and these days IOMMUFD GET_HW_INFO which userspace/VMM will rely on. > Reviewed-by: Vasant Hegde <vasant.hegde@amd.com> > Thanks!
Hi Joao, On 10/18/2023 2:23 PM, Joao Martins wrote: > On 18/10/2023 09:32, Vasant Hegde wrote: >> Joao, >> >> On 9/23/2023 6:55 AM, Joao Martins wrote: >>> Print the feature, much like other kernel-supported features. >>> >>> One can still probe its actual hw support via sysfs, regardless >>> of what the kernel does. >>> >>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com> >>> --- >>> drivers/iommu/amd/init.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c >>> index 45efb7e5d725..b091a3d10819 100644 >>> --- a/drivers/iommu/amd/init.c >>> +++ b/drivers/iommu/amd/init.c >>> @@ -2208,6 +2208,10 @@ static void print_iommu_info(void) >>> >>> if (iommu->features & FEATURE_GAM_VAPIC) >>> pr_cont(" GA_vAPIC"); >>> + if (iommu->features & FEATURE_HASUP) >>> + pr_cont(" HASup"); >>> + if (iommu->features & FEATURE_HDSUP) >>> + pr_cont(" HDSup"); >> >> Note that this has a conflict with iommu/next branch. But it should be fairly >> straight to fix it. Otherwise patch looks good to me. >> > I guess it's this patch, thanks for reminding: > > https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/commit/drivers/iommu/amd/init.c?h=next&id=7b7563a93437ef945c829538da28f0095f1603ec Right. > > But then it's the same problem as the previous patch. The loop above is enterily > reworked, so the code above won't work, and the "iommu->features &" conditionals > needs to be replaced with a check_feature(FEATURE_HDSUP) and > check_feature(FEATURE_HASUP). And depending on the order of pull requests this > is problematic. The previous patch can get away with direct usage of > amd_iommu_efr, but this one sadly no. > > I can skip this patch in particular for v4 and re-submit after -rc1 when > everything is aligned. It is only for user experience about console printing two > strings. Real feature probe is not affected users still have the old sysfs > interface, and these days IOMMUFD GET_HW_INFO which userspace/VMM will rely on. IIUC this can be an independent patch and doesn't have strict dependency on this series itself. May be you can rebase it on top of iommu/next and post it separately? -Vasant
On 18/10/2023 10:03, Vasant Hegde wrote: > Hi Joao, > > On 10/18/2023 2:23 PM, Joao Martins wrote: >> On 18/10/2023 09:32, Vasant Hegde wrote: >>> Joao, >>> >>> On 9/23/2023 6:55 AM, Joao Martins wrote: >>>> Print the feature, much like other kernel-supported features. >>>> >>>> One can still probe its actual hw support via sysfs, regardless >>>> of what the kernel does. >>>> >>>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com> >>>> --- >>>> drivers/iommu/amd/init.c | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c >>>> index 45efb7e5d725..b091a3d10819 100644 >>>> --- a/drivers/iommu/amd/init.c >>>> +++ b/drivers/iommu/amd/init.c >>>> @@ -2208,6 +2208,10 @@ static void print_iommu_info(void) >>>> >>>> if (iommu->features & FEATURE_GAM_VAPIC) >>>> pr_cont(" GA_vAPIC"); >>>> + if (iommu->features & FEATURE_HASUP) >>>> + pr_cont(" HASup"); >>>> + if (iommu->features & FEATURE_HDSUP) >>>> + pr_cont(" HDSup"); >>> >>> Note that this has a conflict with iommu/next branch. But it should be fairly >>> straight to fix it. Otherwise patch looks good to me. >>> >> I guess it's this patch, thanks for reminding: >> >> https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/commit/drivers/iommu/amd/init.c?h=next&id=7b7563a93437ef945c829538da28f0095f1603ec > > Right. > >> >> But then it's the same problem as the previous patch. The loop above is enterily >> reworked, so the code above won't work, and the "iommu->features &" conditionals >> needs to be replaced with a check_feature(FEATURE_HDSUP) and >> check_feature(FEATURE_HASUP). And depending on the order of pull requests this >> is problematic. The previous patch can get away with direct usage of >> amd_iommu_efr, but this one sadly no. >> >> I can skip this patch in particular for v4 and re-submit after -rc1 when >> everything is aligned. It is only for user experience about console printing two >> strings. Real feature probe is not affected users still have the old sysfs >> interface, and these days IOMMUFD GET_HW_INFO which userspace/VMM will rely on. > > IIUC this can be an independent patch and doesn't have strict dependency on this > series itself. May be you can rebase it on top of iommu/next and post it separately? Yeap, pretty much; I've removed this from this series.
On Wed, Oct 18, 2023 at 02:33:09PM +0530, Vasant Hegde wrote: > IIUC this can be an independent patch and doesn't have strict dependency on this > series itself. May be you can rebase it on top of iommu/next and post it separately? No, we can't merge it that way. If this is required the AMD parts have to wait for rc1 Jason
On 18/10/2023 16:52, Jason Gunthorpe wrote: > On Wed, Oct 18, 2023 at 02:33:09PM +0530, Vasant Hegde wrote: > >> IIUC this can be an independent patch and doesn't have strict dependency on this >> series itself. May be you can rebase it on top of iommu/next and post it separately? > > No, we can't merge it that way. If this is required the AMD parts have > to wait for rc1 I've removed this specific patch from the series, and was aiming to post to -rc1 when the dependent parts are settled. It really it just printing 6 letters on the kernel log. It's a tiny thing really, and the rest of the series does not depend this at all. Joao
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c index 45efb7e5d725..b091a3d10819 100644 --- a/drivers/iommu/amd/init.c +++ b/drivers/iommu/amd/init.c @@ -2208,6 +2208,10 @@ static void print_iommu_info(void) if (iommu->features & FEATURE_GAM_VAPIC) pr_cont(" GA_vAPIC"); + if (iommu->features & FEATURE_HASUP) + pr_cont(" HASup"); + if (iommu->features & FEATURE_HDSUP) + pr_cont(" HDSup"); if (iommu->features & FEATURE_SNP) pr_cont(" SNP");
Print the feature, much like other kernel-supported features. One can still probe its actual hw support via sysfs, regardless of what the kernel does. Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- drivers/iommu/amd/init.c | 4 ++++ 1 file changed, 4 insertions(+)