[v2,5/8] x86/mtrr: revert commit 90b926e68f50

  Commit 90b926e68f50 ("x86/pat: Fix pat_x_mtrr_type() for MTRR disabled
case") has introduced a regression with Xen.

Revert the patch.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/mm/pat/memtype.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
  

Hi, this is your Linux kernel regression tracker.

On 09.02.23 08:22, Juergen Gross wrote:
> Commit 90b926e68f50 ("x86/pat: Fix pat_x_mtrr_type() for MTRR disabled
> case") has introduced a regression with Xen.
> 
> Revert the patch.

That regression you refer to is afaics one I'm tracking[1] that was
introduced this cycle. That makes me wonder: could this patch be applied
directly to fix the issue quickly? Or are patches 1 to 4 needed as well
(or the whole series?) to avoid other problems?

Ciao, Thorsten

[1]
https://lore.kernel.org/all/4fe9541e-4d4c-2b2a-f8c8-2d34a7284930@nerdbynature.de/

P.S.: BTW; let me tell regzbot to monitor this thread:

#regzbot ^backmonitor:
https://lore.kernel.org/all/4fe9541e-4d4c-2b2a-f8c8-2d34a7284930@nerdbynature.de/


> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  arch/x86/mm/pat/memtype.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/x86/mm/pat/memtype.c b/arch/x86/mm/pat/memtype.c
> index fb4b1b5e0dea..46de9cf5c91d 100644
> --- a/arch/x86/mm/pat/memtype.c
> +++ b/arch/x86/mm/pat/memtype.c
> @@ -387,8 +387,7 @@ static unsigned long pat_x_mtrr_type(u64 start, u64 end,
>  		u8 mtrr_type, uniform;
>  
>  		mtrr_type = mtrr_type_lookup(start, end, &uniform);
> -		if (mtrr_type != MTRR_TYPE_WRBACK &&
> -		    mtrr_type != MTRR_TYPE_INVALID)
> +		if (mtrr_type != MTRR_TYPE_WRBACK)
>  			return _PAGE_CACHE_MODE_UC_MINUS;
>  
>  		return _PAGE_CACHE_MODE_WB;
  

On 10.02.23 19:59, Linux regression tracking (Thorsten Leemhuis) wrote:
> Hi, this is your Linux kernel regression tracker.
> 
> On 09.02.23 08:22, Juergen Gross wrote:
>> Commit 90b926e68f50 ("x86/pat: Fix pat_x_mtrr_type() for MTRR disabled
>> case") has introduced a regression with Xen.
>>
>> Revert the patch.
> 
> That regression you refer to is afaics one I'm tracking[1] that was
> introduced this cycle. That makes me wonder: could this patch be applied
> directly to fix the issue quickly? Or are patches 1 to 4 needed as well
> (or the whole series?) to avoid other problems?

Patches 1-4 are needed, too, as otherwise the issue claimed to be fixed
with patch 5 would show up again.

I'm working on addressing the comments I've received.


Juergen

> 
> Ciao, Thorsten
> 
> [1]
> https://lore.kernel.org/all/4fe9541e-4d4c-2b2a-f8c8-2d34a7284930@nerdbynature.de/
> 
> P.S.: BTW; let me tell regzbot to monitor this thread:
> 
> #regzbot ^backmonitor:
> https://lore.kernel.org/all/4fe9541e-4d4c-2b2a-f8c8-2d34a7284930@nerdbynature.de/
> 
> 
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>   arch/x86/mm/pat/memtype.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/mm/pat/memtype.c b/arch/x86/mm/pat/memtype.c
>> index fb4b1b5e0dea..46de9cf5c91d 100644
>> --- a/arch/x86/mm/pat/memtype.c
>> +++ b/arch/x86/mm/pat/memtype.c
>> @@ -387,8 +387,7 @@ static unsigned long pat_x_mtrr_type(u64 start, u64 end,
>>   		u8 mtrr_type, uniform;
>>   
>>   		mtrr_type = mtrr_type_lookup(start, end, &uniform);
>> -		if (mtrr_type != MTRR_TYPE_WRBACK &&
>> -		    mtrr_type != MTRR_TYPE_INVALID)
>> +		if (mtrr_type != MTRR_TYPE_WRBACK)
>>   			return _PAGE_CACHE_MODE_UC_MINUS;
>>   
>>   		return _PAGE_CACHE_MODE_WB;
  

On Mon, 13 Feb 2023, Juergen Gross wrote:
> On 10.02.23 19:59, Linux regression tracking (Thorsten Leemhuis) wrote:
> > Hi, this is your Linux kernel regression tracker.
> > 
> > On 09.02.23 08:22, Juergen Gross wrote:
> > > Commit 90b926e68f50 ("x86/pat: Fix pat_x_mtrr_type() for MTRR disabled
> > > case") has introduced a regression with Xen.
> > > 
> > > Revert the patch.
> > 
> > That regression you refer to is afaics one I'm tracking[1] that was
> > introduced this cycle. That makes me wonder: could this patch be applied
> > directly to fix the issue quickly? Or are patches 1 to 4 needed as well
> > (or the whole series?) to avoid other problems?
> 
> Patches 1-4 are needed, too, as otherwise the issue claimed to be fixed
> with patch 5 would show up again.

The (last?) -rc8 version was released yesterday. Would it be possible to 
include at least (only) the revert in mainline so that 6.2 will be 
released with a working storage configuration under Xen?

Otherwise one would have to carry around that single revert manually until 
this patch series has landed in mainline, or convince all the 
distributions to do so :-\

Anyway, thanks for fixing this problem, I did not expect this to be such a 
complicated issue when I reported that thing :-)

Christian.
  

On 13.02.23 12:46, Christian Kujau wrote:
> On Mon, 13 Feb 2023, Juergen Gross wrote:
>> On 10.02.23 19:59, Linux regression tracking (Thorsten Leemhuis) wrote:
>>> Hi, this is your Linux kernel regression tracker.
>>>
>>> On 09.02.23 08:22, Juergen Gross wrote:
>>>> Commit 90b926e68f50 ("x86/pat: Fix pat_x_mtrr_type() for MTRR disabled
>>>> case") has introduced a regression with Xen.
>>>>
>>>> Revert the patch.
>>>
>>> That regression you refer to is afaics one I'm tracking[1] that was
>>> introduced this cycle. That makes me wonder: could this patch be applied
>>> directly to fix the issue quickly? Or are patches 1 to 4 needed as well
>>> (or the whole series?) to avoid other problems?
>>
>> Patches 1-4 are needed, too, as otherwise the issue claimed to be fixed
>> with patch 5 would show up again.
> 
> The (last?) -rc8 version was released yesterday. Would it be possible to
> include at least (only) the revert in mainline so that 6.2 will be
> released with a working storage configuration under Xen?

Hmm, this would make Hyper-V SEV-SNP guests slow again.

I'm not completely against it, but OTOH I'm a little bit biased as the
maintainer of the Xen code. :-)

Michael, would you see major problems with doing the revert before having
the final patches for fixing your issue, too?

> Otherwise one would have to carry around that single revert manually until
> this patch series has landed in mainline, or convince all the
> distributions to do so :-\
> 
> Anyway, thanks for fixing this problem, I did not expect this to be such a
> complicated issue when I reported that thing :-)

Yes, I have opened a can of worms with my MTRR/PAT disentangling.


Juergen
  

From: Juergen Gross <jgross@suse.com>
> 
> On 13.02.23 12:46, Christian Kujau wrote:
> > On Mon, 13 Feb 2023, Juergen Gross wrote:
> >> On 10.02.23 19:59, Linux regression tracking (Thorsten Leemhuis) wrote:
> >>> Hi, this is your Linux kernel regression tracker.
> >>>
> >>> On 09.02.23 08:22, Juergen Gross wrote:
> >>>> Commit 90b926e68f50 ("x86/pat: Fix pat_x_mtrr_type() for MTRR disabled
> >>>> case") has introduced a regression with Xen.
> >>>>
> >>>> Revert the patch.
> >>>
> >>> That regression you refer to is afaics one I'm tracking[1] that was
> >>> introduced this cycle. That makes me wonder: could this patch be applied
> >>> directly to fix the issue quickly? Or are patches 1 to 4 needed as well
> >>> (or the whole series?) to avoid other problems?
> >>
> >> Patches 1-4 are needed, too, as otherwise the issue claimed to be fixed
> >> with patch 5 would show up again.
> >
> > The (last?) -rc8 version was released yesterday. Would it be possible to
> > include at least (only) the revert in mainline so that 6.2 will be
> > released with a working storage configuration under Xen?
> 
> Hmm, this would make Hyper-V SEV-SNP guests slow again.
> 
> I'm not completely against it, but OTOH I'm a little bit biased as the
> maintainer of the Xen code. :-)
> 
> Michael, would you see major problems with doing the revert before having
> the final patches for fixing your issue, too?
> 

I'm OK with doing the revert.  It's probably the right tradeoff for the
broader community because the Hyper-V use case is more narrow and
requires more curation for other reasons.  The use case is the Azure
public cloud, and we can pretty much make sure that one of the solutions
is applied to kernels used with SEV-SNP in that environment.

Michael

> > Otherwise one would have to carry around that single revert manually until
> > this patch series has landed in mainline, or convince all the
> > distributions to do so :-\
> >
> > Anyway, thanks for fixing this problem, I did not expect this to be such a
> > complicated issue when I reported that thing :-)
> 
> Yes, I have opened a can of worms with my MTRR/PAT disentangling.
> 
> 
> Juergen
  

On 13.02.23 18:01, Michael Kelley (LINUX) wrote:
> From: Juergen Gross <jgross@suse.com>
>>
>> On 13.02.23 12:46, Christian Kujau wrote:
>>> On Mon, 13 Feb 2023, Juergen Gross wrote:
>>>> On 10.02.23 19:59, Linux regression tracking (Thorsten Leemhuis) wrote:
>>>>> Hi, this is your Linux kernel regression tracker.
>>>>>
>>>>> On 09.02.23 08:22, Juergen Gross wrote:
>>>>>> Commit 90b926e68f50 ("x86/pat: Fix pat_x_mtrr_type() for MTRR disabled
>>>>>> case") has introduced a regression with Xen.
>>>>>>
>>>>>> Revert the patch.
>>>>>
>>>>> That regression you refer to is afaics one I'm tracking[1] that was
>>>>> introduced this cycle. That makes me wonder: could this patch be applied
>>>>> directly to fix the issue quickly? Or are patches 1 to 4 needed as well
>>>>> (or the whole series?) to avoid other problems?
>>>>
>>>> Patches 1-4 are needed, too, as otherwise the issue claimed to be fixed
>>>> with patch 5 would show up again.
>>>
>>> The (last?) -rc8 version was released yesterday. Would it be possible to
>>> include at least (only) the revert in mainline so that 6.2 will be
>>> released with a working storage configuration under Xen?
>>
>> Hmm, this would make Hyper-V SEV-SNP guests slow again.
>>
>> I'm not completely against it, but OTOH I'm a little bit biased as the
>> maintainer of the Xen code. :-)
>>
>> Michael, would you see major problems with doing the revert before having
>> the final patches for fixing your issue, too?
>>
> 
> I'm OK with doing the revert.  It's probably the right tradeoff for the
> broader community because the Hyper-V use case is more narrow and
> requires more curation for other reasons.  The use case is the Azure
> public cloud, and we can pretty much make sure that one of the solutions
> is applied to kernels used with SEV-SNP in that environment.

Thanks.

Boris, would you take the revert (patch 5 of my series) via x86/urgent, please?


Juergen
  

On Mon, 13 Feb 2023, Juergen Gross wrote:
> Hmm, this would make Hyper-V SEV-SNP guests slow again.
> 
> I'm not completely against it, but OTOH I'm a little bit biased as the
> maintainer of the Xen code. :-)

Understood. I'm a bit puzzled why nobody else reports this, maybe Xen Dom0 
and external USB enclosures are not that common.

> Michael, would you see major problems with doing the revert before having
> the final patches for fixing your issue, too?

If that revert ends up in mainline, feel free to add:

 Tested-by: Christian Kujau <lists@nerdbynature.de> 

...if that makes any difference.

Thanks,
Christian.
  

On 13.02.23 23:54, Christian Kujau wrote:
> On Mon, 13 Feb 2023, Juergen Gross wrote:
>> Hmm, this would make Hyper-V SEV-SNP guests slow again.
>>
>> I'm not completely against it, but OTOH I'm a little bit biased as the
>> maintainer of the Xen code. :-)
> 
> Understood. I'm a bit puzzled why nobody else reports this, maybe Xen Dom0
> and external USB enclosures are not that common.

Not all USB drivers/interfaces have this problem. Your problems are with:

   Intel Corporation 8 Series/C220 Series Chipset Family USB xHCI (rev 05)

while a system I'm working with the following has no problems:

   Intel Corporation 8 Series/C220 Series Chipset Family USB xHCI (rev 04)

The only difference seems to be the revision of your adapter.

> 
>> Michael, would you see major problems with doing the revert before having
>> the final patches for fixing your issue, too?
> 
> If that revert ends up in mainline, feel free to add:
> 
>   Tested-by: Christian Kujau <lists@nerdbynature.de>

Thanks,


Juergen