On Wed, 23 Feb 2022 17:30:34 +0530 Ani Sinha <a...@anisinha.ca> wrote:
> On Wed, Feb 23, 2022 at 2:34 PM Igor Mammedov <imamm...@redhat.com> wrote: > > > > On Thu, 10 Feb 2022 18:58:21 +0530 > > Ani Sinha <a...@anisinha.ca> wrote: > > > > > When adding E820_RESERVED entries we also accidentally allocate dynamic > > > entries. This is incorrect. We should simply return early with the count > > > of > > > the number of reserved entries added. > > > > can you expand commit message to explain what's wrong and > > how problem manifests ... etc. > > The issue has been present for the last 8 years without apparent > visible issues. I think the only issue is that the bug allocates more > memory in the firmware than is actually needed. let me repeat: Why do you think it's an issue or why it's wrong > > > > > > > > > fixes: 7d67110f2d9a6("pc: add etc/e820 fw_cfg file") > > > cc: kra...@redhat.com > > > Signed-off-by: Ani Sinha <a...@anisinha.ca> > > > --- > > > hw/i386/e820_memory_layout.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/hw/i386/e820_memory_layout.c b/hw/i386/e820_memory_layout.c > > > index bcf9eaf837..afb08253a4 100644 > > > --- a/hw/i386/e820_memory_layout.c > > > +++ b/hw/i386/e820_memory_layout.c > > > @@ -31,6 +31,8 @@ int e820_add_entry(uint64_t address, uint64_t length, > > > uint32_t type) > > > entry->type = cpu_to_le32(type); > > > > > > e820_reserve.count = cpu_to_le32(index); > > > + > > > + return index; > > > } > > > > this changes e820_table size/content, which is added by fw_cfg_add_file() > > to fwcfg, > > as result it breaks ABI in case of migration. > > Ugh. So should we keep the bug? or do we add config setting to handle > the ABI breakage. >