Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On 10/07/17 18:38, Eduardo Habkost wrote: > On Mon, Jul 10, 2017 at 05:23:36PM +0200, Igor Mammedov wrote: >> On Mon, 10 Jul 2017 11:53:31 -0300 >> Eduardo Habkost wrote: >> >>> On Mon, Jul 10, 2017 at 10:01:47AM +0200, Igor Mammedov wrote: On Fri, 7 Jul 2017 17:20:25

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On Mon, Jul 10, 2017 at 05:23:36PM +0200, Igor Mammedov wrote: > On Mon, 10 Jul 2017 11:53:31 -0300 > Eduardo Habkost wrote: > > > On Mon, Jul 10, 2017 at 10:01:47AM +0200, Igor Mammedov wrote: > > > On Fri, 7 Jul 2017 17:20:25 +0100 > > > Mark Cave-Ayland

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On Mon, 10 Jul 2017 11:53:31 -0300 Eduardo Habkost wrote: > On Mon, Jul 10, 2017 at 10:01:47AM +0200, Igor Mammedov wrote: > > On Fri, 7 Jul 2017 17:20:25 +0100 > > Mark Cave-Ayland wrote: > > > > > On 07/07/17 16:07, Eduardo Habkost wrote:

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On Mon, Jul 10, 2017 at 10:01:47AM +0200, Igor Mammedov wrote: > On Fri, 7 Jul 2017 17:20:25 +0100 > Mark Cave-Ayland wrote: > > > On 07/07/17 16:07, Eduardo Habkost wrote: > > > > >> looks fine, > > >> > > >> so what I'd do is: > > >> * drop 4/6 > > > > Yes.

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On Mon, Jul 10, 2017 at 09:49:38AM +0200, Igor Mammedov wrote: > On Fri, 7 Jul 2017 12:07:07 -0300 > "Eduardo Habkost" wrote: > > > On Fri, Jul 07, 2017 at 04:44:53PM +0200, Igor Mammedov wrote: > > [...] > > > > > taking in account that fwcfg in not user creatable device

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On Fri, 7 Jul 2017 17:20:25 +0100 Mark Cave-Ayland wrote: > On 07/07/17 16:07, Eduardo Habkost wrote: > > >> looks fine, > >> > >> so what I'd do is: > >> * drop 4/6 > > Yes. > > > Agreed on this point. But: > > > >> * make fw_cfg_find() use ambiguous

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On Fri, 7 Jul 2017 12:07:07 -0300 "Eduardo Habkost" wrote: > On Fri, Jul 07, 2017 at 04:44:53PM +0200, Igor Mammedov wrote: > [...] > > > > taking in account that fwcfg in not user creatable device how about: > > > > > > > > diff --git a/hw/nvram/fw_cfg.c

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On 07/07/17 21:54, Eduardo Habkost wrote: > On Fri, Jul 07, 2017 at 04:30:29PM -0300, Eduardo Habkost wrote: >> On Fri, Jul 07, 2017 at 08:18:20PM +0200, Igor Mammedov wrote: >>> On Fri, 7 Jul 2017 12:09:56 -0300 >>> Eduardo Habkost wrote: >>> On Fri, Jul 07, 2017 at

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On Fri, Jul 07, 2017 at 04:30:29PM -0300, Eduardo Habkost wrote: > On Fri, Jul 07, 2017 at 08:18:20PM +0200, Igor Mammedov wrote: > > On Fri, 7 Jul 2017 12:09:56 -0300 > > Eduardo Habkost wrote: > > > > > On Fri, Jul 07, 2017 at 04:58:17PM +0200, Igor Mammedov wrote: > > > >

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On Fri, Jul 07, 2017 at 08:18:20PM +0200, Igor Mammedov wrote: > On Fri, 7 Jul 2017 12:09:56 -0300 > Eduardo Habkost wrote: > > > On Fri, Jul 07, 2017 at 04:58:17PM +0200, Igor Mammedov wrote: > > > On Fri, 7 Jul 2017 10:13:00 -0300 > > > "Eduardo Habkost"

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On Fri, 7 Jul 2017 12:09:56 -0300 Eduardo Habkost wrote: > On Fri, Jul 07, 2017 at 04:58:17PM +0200, Igor Mammedov wrote: > > On Fri, 7 Jul 2017 10:13:00 -0300 > > "Eduardo Habkost" wrote: > [...] > > > I don't disagree with adding the assert(), but it

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On 07/07/17 16:07, Eduardo Habkost wrote: >> looks fine, >> >> so what I'd do is: >> * drop 4/6 Yes. > Agreed on this point. But: > >> * make fw_cfg_find() use ambiguous argument and error_abort if ambiguous == >> true During my latest tests I've found that everything works fine without

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On 07/07/17 15:48, Eduardo Habkost wrote: >>> I don't see what needs to be fixed. It is not a bug to leave >>> fw_cfg in /machine/unattached, as long as fw_cfg_find() works >>> properly. >> >> Yeah. I wonder if I've been leading myself astray down the wrong path >> here? Let me do some more

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On 07/07/17 14:13, Eduardo Habkost wrote: >>> This should be the same behaviour as before, i.e. a NULL means that the >>> fw_cfg device couldn't be found. It seems intuitive to me from the name >>> of the function exactly what it does, so I don't find the lack of >>> comment too confusing. Does

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On Fri, Jul 07, 2017 at 04:58:17PM +0200, Igor Mammedov wrote: > On Fri, 7 Jul 2017 10:13:00 -0300 > "Eduardo Habkost" wrote: [...] > > I don't disagree with adding the assert(), but it looks like > > making fw_cfg_find() return NULL if there are multiple devices > > can be

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On Fri, Jul 07, 2017 at 04:44:53PM +0200, Igor Mammedov wrote: [...] > > > taking in account that fwcfg in not user creatable device how about: > > > > > > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > > > index 316fca9..8f6eef8 100644 > > > --- a/hw/nvram/fw_cfg.c > > > +++

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On Fri, 7 Jul 2017 10:13:00 -0300 "Eduardo Habkost" wrote: > On Fri, Jul 07, 2017 at 01:33:20PM +0200, Igor Mammedov wrote: > > On Tue, 4 Jul 2017 19:08:44 +0100 > > Mark Cave-Ayland wrote: > > > > > On 03/07/17 10:39, Igor Mammedov wrote:

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On Fri, Jul 07, 2017 at 04:44:53PM +0200, Igor Mammedov wrote: [...] > > 3) Since these checks are done at realize time, we can provide nice > > friendly messages back to the developer to tell them what needs to be fixed > error_set(error_abort, ...) should work nice for fwcfg usecase, > you get

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On Fri, 7 Jul 2017 14:12:19 +0100 Mark Cave-Ayland wrote: > On 07/07/17 12:33, Igor Mammedov wrote: > > > On Tue, 4 Jul 2017 19:08:44 +0100 > > Mark Cave-Ayland wrote: > > > >> On 03/07/17 10:39, Igor Mammedov wrote: > >> > >>>

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On Fri, Jul 07, 2017 at 02:54:49PM +0100, Mark Cave-Ayland wrote: > On 07/07/17 14:26, Eduardo Habkost wrote: > > (cut) > > >> However this causes us a problem: if you instantiate the fw_cfg yourself > >> with qdev_create()...qdev_init_nofail() then you can potentially access > >> the underlying

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On 07/07/17 14:26, Eduardo Habkost wrote: (cut) >> However this causes us a problem: if you instantiate the fw_cfg yourself >> with qdev_create()...qdev_init_nofail() then you can potentially access >> the underlying MemoryRegions directly and wire up the device without >> attaching it to the

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On 07/07/17 12:33, Igor Mammedov wrote: > On Tue, 4 Jul 2017 19:08:44 +0100 > Mark Cave-Ayland wrote: > >> On 03/07/17 10:39, Igor Mammedov wrote: >> >>> On Thu, 29 Jun 2017 15:07:19 +0100 >>> Mark Cave-Ayland wrote: >>>

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On Fri, Jul 07, 2017 at 02:12:19PM +0100, Mark Cave-Ayland wrote: > On 07/07/17 12:33, Igor Mammedov wrote: > > > On Tue, 4 Jul 2017 19:08:44 +0100 > > Mark Cave-Ayland wrote: > > > >> On 03/07/17 10:39, Igor Mammedov wrote: > >> > >>> On Thu, 29 Jun 2017 15:07:19

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On Fri, Jul 07, 2017 at 01:33:20PM +0200, Igor Mammedov wrote: > On Tue, 4 Jul 2017 19:08:44 +0100 > Mark Cave-Ayland wrote: > > > On 03/07/17 10:39, Igor Mammedov wrote: > > > > > On Thu, 29 Jun 2017 15:07:19 +0100 > > > Mark Cave-Ayland

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On Tue, 4 Jul 2017 19:08:44 +0100 Mark Cave-Ayland wrote: > On 03/07/17 10:39, Igor Mammedov wrote: > > > On Thu, 29 Jun 2017 15:07:19 +0100 > > Mark Cave-Ayland wrote: > > > >> When looking to instantiate a TYPE_FW_CFG_MEM or

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On 03/07/17 10:39, Igor Mammedov wrote: > On Thu, 29 Jun 2017 15:07:19 +0100 > Mark Cave-Ayland wrote: > >> When looking to instantiate a TYPE_FW_CFG_MEM or TYPE_FW_CFG_IO device to be >> able to wire it up differently, it is much more convenient for the caller to

Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

On Thu, 29 Jun 2017 15:07:19 +0100 Mark Cave-Ayland wrote: > When looking to instantiate a TYPE_FW_CFG_MEM or TYPE_FW_CFG_IO device to be > able to wire it up differently, it is much more convenient for the caller to > instantiate the device and have the fw_cfg

[Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers

When looking to instantiate a TYPE_FW_CFG_MEM or TYPE_FW_CFG_IO device to be able to wire it up differently, it is much more convenient for the caller to instantiate the device and have the fw_cfg default files already preloaded during realize. Move fw_cfg_init1() to the end of both the