Re: [PATCH] x86/mm/sme: fix the kdump kernel breakage on SME system when CONFIG_IMA_KEXEC=y

[Baoquan He <bhe@redhat.com>]

On 08/27/24 at 01:41pm, Dave Young wrote:
> On Tue, 27 Aug 2024 at 13:28, Baoquan He <bhe@redhat.com> wrote:
> >
> > On 08/26/24 at 09:24am, Tom Lendacky wrote:
> > > On 8/25/24 21:44, Baoquan He wrote:
......
> > > > ---
> > > >  arch/x86/mm/ioremap.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> > > > index aa7d279321ea..7953c4a1d28d 100644
> > > > --- a/arch/x86/mm/ioremap.c
> > > > +++ b/arch/x86/mm/ioremap.c
> > > > @@ -717,7 +717,7 @@ static bool __init early_memremap_is_setup_data(resource_size_t phys_addr,
> > > >             paddr_next = data->next;
> > > >             len = data->len;
> > > >
> > > > -           if ((phys_addr > paddr) && (phys_addr < (paddr + len))) {
> > > > +           if ((phys_addr > paddr) && (phys_addr < (paddr + size + len))) {
> > >
> > > I don't think this is correct. You are adding the requested size to the
> > > length of the setup data element. The length is the true length of the
> > > setup data and should not be increased.
> >
> > I talked to Dave, he reminded me that people could mix the passed in
> > parameter 'size' and the local variable 'size' defined inside the while
> > loop, not sure which 'size' you are referring to.
> >
> Baoquan, you are right, but I think I mistakenly read the code in
> memremap_is_setup_data instead of early_memremap_is_setup_data.  You
> can check the memremap_is_setup_data, no "size = sizeof (*data)",  so
> these two functions could both need fixes.

Agree, memremap_is_setup_data() has the same drawback in code.

> 
> Otherwise it would be better to change the function internal variable
> name, it could cause confusion even if the actual result is correct.

Ok, will consider to change when spinning v2.