| Message ID | 20220603085920.604323-1-kraxel@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | microvm: turn off io reservations for pcie root ports | expand |
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>
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
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
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.
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
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.
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
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.
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 = {
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(+)