[PATCH] microvm: turn off io reservations for pcie root ports

[PATCH] microvm: turn off io reservations for pcie root ports

[Gerd Hoffmann]

The pcie host bridge has no io window on microvm,
so io reservations will not work.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/i386/microvm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 4b3b1dd262f1..f01d972f5d28 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -757,6 +757,12 @@ static void microvm_class_init(ObjectClass *oc, void *data)
         "Set off to disable adding virtio-mmio devices to the kernel cmdline");
 
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
+
+    /*
+     * pcie host bridge (gpex) on microvm has no io address window,
+     * so reserving io space is not going to work.  Turn it off.
+     */
+    object_register_sugar_prop("pcie-root-port", "io-reserve", "0", true);
 }
 
 static const TypeInfo microvm_machine_info = {
-- 
2.36.1

Re: [PATCH] microvm: turn off io reservations for pcie root ports

[Philippe Mathieu-Daudé via]

On 3/6/22 10:59, Gerd Hoffmann wrote:
> The pcie host bridge has no io window on microvm,
> so io reservations will not work.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   hw/i386/microvm.c | 6 ++++++
>   1 file changed, 6 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH] microvm: turn off io reservations for pcie root ports

[Sergio Lopez]

[-- Attachment #1: Type: text/plain, Size: 332 bytes --]

On Fri, Jun 03, 2022 at 10:59:20AM +0200, Gerd Hoffmann wrote:
> The pcie host bridge has no io window on microvm,
> so io reservations will not work.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/i386/microvm.c | 6 ++++++
>  1 file changed, 6 insertions(+)

Reviewed-by: Sergio Lopez <slp@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] microvm: turn off io reservations for pcie root ports

[Michael S. Tsirkin]

On Fri, Jun 03, 2022 at 10:59:20AM +0200, Gerd Hoffmann wrote:
> The pcie host bridge has no io window on microvm,
> so io reservations will not work.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

I don't much like overriding user like this. We end up users
setting it to silly values and then if we do want to
support this things just break. Thoughts?

> ---
>  hw/i386/microvm.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
> index 4b3b1dd262f1..f01d972f5d28 100644
> --- a/hw/i386/microvm.c
> +++ b/hw/i386/microvm.c
> @@ -757,6 +757,12 @@ static void microvm_class_init(ObjectClass *oc, void *data)
>          "Set off to disable adding virtio-mmio devices to the kernel cmdline");
>  
>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
> +
> +    /*
> +     * pcie host bridge (gpex) on microvm has no io address window,
> +     * so reserving io space is not going to work.  Turn it off.
> +     */
> +    object_register_sugar_prop("pcie-root-port", "io-reserve", "0", true);
>  }
>  
>  static const TypeInfo microvm_machine_info = {
> -- 
> 2.36.1

Re: [PATCH] microvm: turn off io reservations for pcie root ports

[Gerd Hoffmann]

On Wed, Jun 08, 2022 at 12:06:17PM -0400, Michael S. Tsirkin wrote:
> On Fri, Jun 03, 2022 at 10:59:20AM +0200, Gerd Hoffmann wrote:
> > The pcie host bridge has no io window on microvm,
> > so io reservations will not work.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> I don't much like overriding user like this. We end up users
> setting it to silly values and then if we do want to
> support this things just break. Thoughts?

Well, it just looked like the simplest way to tell the firmware that
io reservations are pointless.  Do you have a better idea?

take care,
  Gerd

Re: [PATCH] microvm: turn off io reservations for pcie root ports

[Michael S. Tsirkin]

On Thu, Jun 09, 2022 at 09:28:38AM +0200, Gerd Hoffmann wrote:
> On Wed, Jun 08, 2022 at 12:06:17PM -0400, Michael S. Tsirkin wrote:
> > On Fri, Jun 03, 2022 at 10:59:20AM +0200, Gerd Hoffmann wrote:
> > > The pcie host bridge has no io window on microvm,
> > > so io reservations will not work.
> > > 
> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > 
> > I don't much like overriding user like this. We end up users
> > setting it to silly values and then if we do want to
> > support this things just break. Thoughts?
> 
> Well, it just looked like the simplest way to tell the firmware that
> io reservations are pointless.  Do you have a better idea?
> 
> take care,
>   Gerd

Fail if user supplies values we can't support.

-- 
MST

Re: [PATCH] microvm: turn off io reservations for pcie root ports

[Gerd Hoffmann]

On Mon, Jun 27, 2022 at 06:37:50PM -0400, Michael S. Tsirkin wrote:
> On Thu, Jun 09, 2022 at 09:28:38AM +0200, Gerd Hoffmann wrote:
> > On Wed, Jun 08, 2022 at 12:06:17PM -0400, Michael S. Tsirkin wrote:
> > > On Fri, Jun 03, 2022 at 10:59:20AM +0200, Gerd Hoffmann wrote:
> > > > The pcie host bridge has no io window on microvm,
> > > > so io reservations will not work.
> > > > 
> > > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > > 
> > > I don't much like overriding user like this. We end up users
> > > setting it to silly values and then if we do want to
> > > support this things just break. Thoughts?
> > 
> > Well, it just looked like the simplest way to tell the firmware that
> > io reservations are pointless.  Do you have a better idea?
> > 
> > take care,
> >   Gerd
> 
> Fail if user supplies values we can't support.

Well, it is the *default* value which doesn't work on microvm.

take care,
  Gerd

Re: [PATCH] microvm: turn off io reservations for pcie root ports

[Michael S. Tsirkin]

On Wed, Jun 29, 2022 at 09:10:23AM +0200, Gerd Hoffmann wrote:
> On Mon, Jun 27, 2022 at 06:37:50PM -0400, Michael S. Tsirkin wrote:
> > On Thu, Jun 09, 2022 at 09:28:38AM +0200, Gerd Hoffmann wrote:
> > > On Wed, Jun 08, 2022 at 12:06:17PM -0400, Michael S. Tsirkin wrote:
> > > > On Fri, Jun 03, 2022 at 10:59:20AM +0200, Gerd Hoffmann wrote:
> > > > > The pcie host bridge has no io window on microvm,
> > > > > so io reservations will not work.
> > > > > 
> > > > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > > > 
> > > > I don't much like overriding user like this. We end up users
> > > > setting it to silly values and then if we do want to
> > > > support this things just break. Thoughts?
> > > 
> > > Well, it just looked like the simplest way to tell the firmware that
> > > io reservations are pointless.  Do you have a better idea?
> > > 
> > > take care,
> > >   Gerd
> > 
> > Fail if user supplies values we can't support.
> 
> Well, it is the *default* value which doesn't work on microvm.
> 
> take care,
>   Gerd

Changing defaults is ok of course. Let's just not override the user.

-- 
MST

Re: [PATCH] microvm: turn off io reservations for pcie root ports

[Gerd Hoffmann]

On Wed, Jun 29, 2022 at 03:16:17AM -0400, Michael S. Tsirkin wrote:
> On Wed, Jun 29, 2022 at 09:10:23AM +0200, Gerd Hoffmann wrote:
> > On Mon, Jun 27, 2022 at 06:37:50PM -0400, Michael S. Tsirkin wrote:
> > > On Thu, Jun 09, 2022 at 09:28:38AM +0200, Gerd Hoffmann wrote:
> > > > On Wed, Jun 08, 2022 at 12:06:17PM -0400, Michael S. Tsirkin wrote:
> > > > > On Fri, Jun 03, 2022 at 10:59:20AM +0200, Gerd Hoffmann wrote:
> > > > > > The pcie host bridge has no io window on microvm,
> > > > > > so io reservations will not work.
> > > > > > 
> > > > > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > > > > 
> > > > > I don't much like overriding user like this. We end up users
> > > > > setting it to silly values and then if we do want to
> > > > > support this things just break. Thoughts?
> > > > 
> > > > Well, it just looked like the simplest way to tell the firmware that
> > > > io reservations are pointless.  Do you have a better idea?
> > > > 
> > > > take care,
> > > >   Gerd
> > > 
> > > Fail if user supplies values we can't support.
> > 
> > Well, it is the *default* value which doesn't work on microvm.
> > 
> > take care,
> >   Gerd
> 
> Changing defaults is ok of course. Let's just not override the user.

Ok, so I could use a compat property instead and change the default
for microvm that way.  That would allow the user set any value it
wants.

I still don't see the point though.  There is only a single value which
makes sense on microvm.  Which is zero.  The only effect the user could
archive is make the firmware throwing errors ...

take care,
  Gerd

Re: [PATCH] microvm: turn off io reservations for pcie root ports

[Michael S. Tsirkin]

On Wed, Jun 29, 2022 at 02:20:50PM +0200, Gerd Hoffmann wrote:
> On Wed, Jun 29, 2022 at 03:16:17AM -0400, Michael S. Tsirkin wrote:
> > On Wed, Jun 29, 2022 at 09:10:23AM +0200, Gerd Hoffmann wrote:
> > > On Mon, Jun 27, 2022 at 06:37:50PM -0400, Michael S. Tsirkin wrote:
> > > > On Thu, Jun 09, 2022 at 09:28:38AM +0200, Gerd Hoffmann wrote:
> > > > > On Wed, Jun 08, 2022 at 12:06:17PM -0400, Michael S. Tsirkin wrote:
> > > > > > On Fri, Jun 03, 2022 at 10:59:20AM +0200, Gerd Hoffmann wrote:
> > > > > > > The pcie host bridge has no io window on microvm,
> > > > > > > so io reservations will not work.
> > > > > > > 
> > > > > > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > > > > > 
> > > > > > I don't much like overriding user like this. We end up users
> > > > > > setting it to silly values and then if we do want to
> > > > > > support this things just break. Thoughts?
> > > > > 
> > > > > Well, it just looked like the simplest way to tell the firmware that
> > > > > io reservations are pointless.  Do you have a better idea?
> > > > > 
> > > > > take care,
> > > > >   Gerd
> > > > 
> > > > Fail if user supplies values we can't support.
> > > 
> > > Well, it is the *default* value which doesn't work on microvm.
> > > 
> > > take care,
> > >   Gerd
> > 
> > Changing defaults is ok of course. Let's just not override the user.
> 
> Ok, so I could use a compat property instead and change the default
> for microvm that way.  That would allow the user set any value it
> wants.

And if you like check the value and fail init if not 0.

> I still don't see the point though.  There is only a single value which
> makes sense on microvm.  Which is zero.  The only effect the user could
> archive is make the firmware throwing errors ...
> 
> take care,
>   Gerd

My concern is simple: if right now we override the value then
some users might set it to != 0 by mistake. Then if down the road
we want to intepret != 0 in some way, these setups
will start failing. I don't claim to see a use-case why we'd want to
but it's hard to predict the future with certainty.

-- 
MST