On 08/27/24 at 01:41pm, Dave Young wrote: > On Tue, 27 Aug 2024 at 13:28, Baoquan He <bhe@xxxxxxxxxx> 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. _______________________________________________ kexec mailing list kexec@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/kexec