Re: [PATCH] pci: make ROM memory resizable

[Vladimir Sementsov-Ogievskiy]

On 25.04.23 00:06, Michael S. Tsirkin wrote:
> On Tue, Apr 25, 2023 at 12:02:49AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > On 24.04.23 23:45, Michael S. Tsirkin wrote:
> > > On Mon, Apr 24, 2023 at 04:42:00PM -0400, Michael S. Tsirkin wrote:
> > > > On Mon, Apr 24, 2023 at 11:36:47PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > > > > On migration, on target we load local ROM file. But actual ROM content
> > > > > migrates through migration channel. Original ROM content from local
> > > > > file doesn't matter. But when size mismatch - we have an error like
> > > > >
> > > > >    Size mismatch: 0000:00:03.0/virtio-net-pci.rom: 0x40000 != 0x80000: Invalid 
> > > > > argument
> > > > >
> > > > > Let's just allow resizing of ROM memory. This way migration is not
> > > > > relate on local ROM file on target node which is loaded by default but
> > > > > is not actually needed.
> > > > >
> > > > > Signed-off-by: Vladimir Sementsov-Ogievskiy<vsementsov@yandex-team.ru>
> > > Also isn't ROM size reflected in config space etc?
> > > I don't remember code that would update that on migration.
> >
> > Thanks a lot for fast answers!
> >
> > Hmm. I'm a newbie in these things.
> >
> > But yes, I noted, that my patch helps, if, for example jump from 200K to 500K 
> > zero-filled ROM file. But if jump to 600K, migration fails with
> >
> > (qemu) qemu: get_pci_config_device: Bad config data: i=0x32 read: b8 device: 0 
> > cmask: ff wmask: f0 w1cmask:0
> > qemu: Failed to load PCIDevice:config
> > qemu: Failed to load virtio-net:virtio
> > qemu: error while loading state for instance 0x0 of device 
> > '0000:00:03.0/virtio-net'
> > qemu: load of migration failed: Invalid argument
> >
> >
> > I thought, that, maybe, romfile for this device just mustn't be more than 512K 
> > where config starts. But now I think that it's exactly the problem you are 
> > saying about.
> >
> >
> > I know also, that there were another step around this problem: 08b1df8ff463e72b087 
> > "pci: add romsize property".. But it doesn't help when you already have a 
> > running instance with small ROM and want to migrate it to the node where you have 
> > corresponding local ROM file updated to new package with bigger size.
>
> set romsize on destination?

This does not work if you try to set smaller size, it say:

romfile "b" (409600 bytes) is too large for ROM size 262144

so we need additional option like romalloc=on, that say: don't load any file but just 
allocate ROM by romsize option. Or just handle romfile="",romsize=SOME_SIZE in 
this way.

But I'm still interested in possibility to avoid any additional option on 
target.

> > Hmm. So, simply reuse "resizable" memory blocks doesn't help. And I need more 
> > precise reinitialization of device on load of incoming migration..
> >
> > --
> > Best regards,
> > Vladimir

--
Best regards,
Vladimir