MIME-Version: 1.0
References: <4fe9541e-4d4c-2b2a-f8c8-2d34a7284930@nerdbynature.de>
 <f22551ea-0694-2838-4a3f-f60d8d93fa64@leemhuis.info> <8f132803-f496-f33a-d2ab-b47fd5af0b88@nerdbynature.de>
 <0f441adb-f565-7a37-d9e5-3b14f6f7aea6@suse.com> <f02c49da-0377-97b9-9438-9e0ddbfbcc6d@nerdbynature.de>
 <CAHk-=wg1yXaX+Ut4uctf7x1WrZ4WW9hjSCr1VACwZtkZT9frFw@mail.gmail.com> <Y9+tDmLAlqg37TS+@zn.tnic>
In-Reply-To: <Y9+tDmLAlqg37TS+@zn.tnic>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Sun, 5 Feb 2023 12:21:42 -0800
Message-ID: <CAHk-=wgC_MEFnnzUGN4q9pmhxV+eFV1Oo=W2j1J69YhJF5EDtw@mail.gmail.com>
Subject: Re: External USB disks not recognized with v6.1.8 when using Xen
To:     Borislav Petkov <bp@alien8.de>
Cc:     Christian Kujau <lists@nerdbynature.de>,
        Juergen Gross <jgross@suse.com>,
        Michael Kelley <mikelley@microsoft.com>,
        linux-kernel@vger.kernel.org, Greg KH <gregkh@linuxfoundation.org>,
        Linux regressions mailing list <regressions@lists.linux.dev>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sun, Feb 5, 2023 at 5:20 AM Borislav Petkov <bp@alien8.de> wrote:
>
> @@ -53,7 +53,8 @@ static inline u8 mtrr_type_lookup(u64 addr,
>         /*
>          * Return no-MTRRs:
>          */
> -       return MTRR_TYPE_INVALID;
> +       *uniform = 1;
> +       return MTRR_TYPE_UNCACHABLE;

So this is the one I'd almost leave alone.

Because this is not a "there are no MTRR's" situation, this is a "I
haven't enabled CONFIG_MTRR, so I don't _know_ if there are any MTRR's
or not.

And returning MTRR_TYPE_UNCACHABLE will then disable things like
largepages etc, so this change would effectively mean that if
CONFIG_MTRR is off, it would turn off hugepage support too.

But maybe that was the only thing that cared, and we have:

> @@ -721,8 +721,9 @@ int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot)
>         u8 mtrr, uniform;
>
>         mtrr = mtrr_type_lookup(addr, addr + PUD_SIZE, &uniform);
> -       if ((mtrr != MTRR_TYPE_INVALID) && (!uniform) &&
> -           (mtrr != MTRR_TYPE_WRBACK))
> +       if (mtrr != MTRR_TYPE_UNCACHABLE &&
> +           mtrr != MTRR_TYPE_WRBACK &&
> +           !uniform)
>                 return 0;

Here you make up for it, but I don't actually understand why these
checks exist at all.

I *think* that what the check should do is just check for uniformity.

Why would the largepage code otherwise care?

Other MTRR types are explicitly fine, and I think things like the X
server might even want to do write-combining with large pages etc.

So I think the hugepage code should only do

     if (!uniform)
          return 0;

or there should be some explanation for why those types are special?

>> @@ -748,8 +749,9 @@ int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot)
>         u8 mtrr, uniform;
>
>         mtrr = mtrr_type_lookup(addr, addr + PMD_SIZE, &uniform);
> -       if ((mtrr != MTRR_TYPE_INVALID) && (!uniform) &&
> -           (mtrr != MTRR_TYPE_WRBACK)) {
> +       if (mtrr != MTRR_TYPE_UNCACHABLE &&
> +           mtrr != MTRR_TYPE_WRBACK &&
> +           !uniform) {

Same here.

Again, I *think* that the reason it used to do that "check two types"
thing is simply because "uniform" wasn't set correctly.

But I don't know.

                Linus