MIME-Version: 1.0
References: <Y9wYTnwXVwg/3Dv3@osiris>
In-Reply-To: <Y9wYTnwXVwg/3Dv3@osiris>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Thu, 2 Feb 2023 13:01:21 -0800
Message-ID: <CAHk-=wg3u9fG-Oy_NV+w2H7pFUWmfyX5JXHGvxynxA9b1sNZGw@mail.gmail.com>
Subject: Re: [GIT PULL] s390 fixes for 6.2-rc7
To:     Heiko Carstens <hca@linux.ibm.com>
Cc:     Vasily Gorbik <gor@linux.ibm.com>,
        Alexander Gordeev <agordeev@linux.ibm.com>,
        Wim Van Sebroeck <wim@linux-watchdog.org>,
        Guenter Roeck <linux@roeck-us.net>,
        linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Feb 2, 2023 at 12:08 PM Heiko Carstens <hca@linux.ibm.com> wrote:
>
> - With CONFIG_VMAP_STACK enabled it is not possible to load the s390
>   specific diag288_wdt watchdog module. Reason is that a pointer to a
>   string is passed to an inline assembly; this string however is located on
>   the stack, while the instruction within the inline assembly expects a
>   physicial address. Fix this by copying the string to a kmalloc'ed buffer.

Ugh. I have pulled this, but I think that fix is disgusting.

I think the kmalloc/kfree should have been done inside __diag288_vm()
itself, where it actually does that whole "virt_to_phys()" too.

Now there are three callers of __diag288_vm(), and they all do that
ugly kmalloc game with no comment about why it's needed.

If __diag288_vm() just did it itself, and had a comment right next to
the virt_to_phys() about why, that would look a lot better, I think.

That said, I don't know the code, and maybe there was some reason to
do it this way. As mentioned, I've pulled it, I just don't
particularly love the patch.

              Linus