Re: [Qemu-devel] [PATCH] pflash_cfi01: fix per device sector length in CFI table

[Dr. David Alan Gilbert]

* Andrew Jones (address@hidden) wrote:
> On Thu, Jan 12, 2017 at 10:42:41AM +0000, Peter Maydell wrote:
> > On 12 January 2017 at 10:35, David Engraf <address@hidden> wrote:
> > > The CFI entry for sector length must be set to sector length per device.
> > > This is important for boards using multiple devices like the ARM Vexpress
> > > board (width = 4, device-width = 2).
> > >
> > > Linux and u-boots calculate the size ratio by dividing both values:
> > >
> > > size_ratio = info->portwidth / info->chipwidth;
> > >
> > > After that the sector length will be multiplied by the size_ratio, thus 
> > > the
> > > CFI entry for sector length is doubled. When Linux or u-boot send a sector
> > > erase, they expect to erase the doubled sector length, but QEMU only 
> > > erases
> > > the board specified sector length.
> > >
> > > This patch fixes the sector length in the CFI table to match the length 
> > > per
> > > device, equal to blocks_per_device.
> > 
> > Thanks for the patch. I haven't checked against the pflash spec yet,
> > but this looks like it's probably the right thing.
> > 
> > The only two machines which use a setup with multiple devices (ie
> > which specify device_width to the pflash_cfi01) are vexpress and virt.
> > For all other machines this patch leaves the behaviour unchanged.
> > 
> > Q: do we need to have some kind of nasty hack so that pre-2.9 virt
> > still gets the old broken values in the CFI table, for version and
> > migration compatibility? Ccing Drew for an opinion...
> >
> 
> I'm pretty sure we need the nasty hack, but I'm also Ccing David for
> his opinion.

Hmm I don't understand enough about pflash to understand the change here;
but generally if you need to keep something unchanged for older
machine types, add a property and then set that property in
the compatibility macros.

See include/hw/compat.h, I think you'd add the entry to HW_COMPAT_2_8
and follow a simple example like  old_msi_addr on intel-hda.c,
that should get picked up by aarch, x86, ppc etc versioned machine types.

Dave


> Thanks,
> drew
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK