Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On Wed, Dec 04, 2013 at 11:48:03AM -0500, Konrad Rzeszutek Wilk wrote: > > > which is hardly nice. This patch fixes this by having each > > > PV driver check for: > > > - if running in PV, then it is fine to execute (as that is their > > >native environment). > > > - if running in HVM, check if user wanted 'xen_emul_unplug=never', > > >in which case bail out and don't load PV drivers. > > > - if running in HVM, and if PCI device 5853:0001 (xen_platform_pci) > > >does not exist, then bail out and not load PV drivers. > > > > > > P.S. > > > Ian Campbell suggested getting rid of 'xen_platform_pci_unplug' > > > but unfortunatly the xen-blkfront driver is using it, so we > > > cannot do it. > > > > > > Reported-by: Sander Eikelenboom > > Reported-by: Anthony PERARD > > > Reported-by: Fabio Fantoni > > > > For input: > > > > Acked-by: Dmitry Torokhov > > Thank you. I need to do some extra changes to the other subsystems but > for the fb/kbd itshould be still throught the same function as this > patch has exposed. I will repost it and include your Ack if you are OK > with that? Sure. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
> > which is hardly nice. This patch fixes this by having each
> > PV driver check for:
> > - if running in PV, then it is fine to execute (as that is their
> >native environment).
> > - if running in HVM, check if user wanted 'xen_emul_unplug=never',
> >in which case bail out and don't load PV drivers.
> > - if running in HVM, and if PCI device 5853:0001 (xen_platform_pci)
> >does not exist, then bail out and not load PV drivers.
> >
> > P.S.
> > Ian Campbell suggested getting rid of 'xen_platform_pci_unplug'
> > but unfortunatly the xen-blkfront driver is using it, so we
> > cannot do it.
> >
> > Reported-by: Sander Eikelenboom > Reported-by: Anthony PERARD
> > Reported-by: Fabio Fantoni
>
> For input:
>
> Acked-by: Dmitry Torokhov
Thank you. I need to do some extra changes to the other subsystems but
for the fb/kbd itshould be still throught the same function as this
patch has exposed. I will repost it and include your Ack if you are OK
with that?
>
> ...
>
> > +#if defined(CONFIG_XEN_PVHVM)
> > +extern bool xen_has_pv_devices(void);
> > +#else
> > +static inline bool xen_has_pv_devices(void)
> > +{
> > +#if defined(CONFIG_XEN)
> > + return true;
> > +#else
> > + return false;
> > +#endif
>
> return IS_ENABLED(CONFIG_XEN);
>
> ?
Oh, awesome! Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
Hi Konrad,
On Tue, Dec 03, 2013 at 04:14:06PM -0500, Konrad Rzeszutek Wilk wrote:
> The user has the option of disabling the platform driver:
> 00:02.0 Unassigned class [ff80]: XenSource, Inc. Xen Platform Device (rev 01)
>
> which is used to unplug the emulated drivers (IDE, Realtek 8169, etc)
> and allow the PV drivers to take over. If the user wishes
> to disable that they can set:
>
> xen_platform_pci=0
> (in the guest config file)
>
> or
> xen_emul_unplug=never
> (on the Linux command line)
>
> except it does not work properly. The PV drivers still try to
> load and since the Xen platform driver is not run - and it
> has not initialized the grant tables, most of the PV drivers
> stumble upon:
>
> input: Xen Virtual Keyboard as /devices/virtual/input/input5
> input: Xen Virtual Pointer as /devices/virtual/input/input6M
> [ cut here ]
> kernel BUG at /home/konrad/ssd/konrad/linux/drivers/xen/grant-table.c:1206!
> invalid opcode: [#1] SMP
> Modules linked in: xen_kbdfront(+) xenfs xen_privcmd
> CPU: 6 PID: 1389 Comm: modprobe Not tainted
> 3.13.0-rc1upstream-00021-ga6c892b-dirty #1
> Hardware name: Xen HVM domU, BIOS 4.4-unstable 11/26/2013
> RIP: 0010:[] []
> get_free_entries+0x2e0/0x300
> Call Trace:
> [] ? evdev_connect+0x1e3/0x240
> [] gnttab_grant_foreign_access+0x2e/0x70
> [] xenkbd_connect_backend+0x41/0x290 [xen_kbdfront]
> [] xenkbd_probe+0x2f2/0x324 [xen_kbdfront]
> [] xenbus_dev_probe+0x77/0x130
> [] xenbus_frontend_dev_probe+0x47/0x50
> [] driver_probe_device+0x89/0x230
> [] __driver_attach+0x9b/0xa0
> [] ? driver_probe_device+0x230/0x230
> [] ? driver_probe_device+0x230/0x230
> [] bus_for_each_dev+0x8c/0xb0
> [] driver_attach+0x19/0x20
> [] bus_add_driver+0x1a0/0x220
> [] driver_register+0x5f/0xf0
> [] xenbus_register_driver_common+0x15/0x20
> [] xenbus_register_frontend+0x23/0x40
> [] ? 0xa0014fff
> [] xenkbd_init+0x2b/0x1000 [xen_kbdfront]
> [] do_one_initcall+0x49/0x170
>
> .. snip..
>
> which is hardly nice. This patch fixes this by having each
> PV driver check for:
> - if running in PV, then it is fine to execute (as that is their
>native environment).
> - if running in HVM, check if user wanted 'xen_emul_unplug=never',
>in which case bail out and don't load PV drivers.
> - if running in HVM, and if PCI device 5853:0001 (xen_platform_pci)
>does not exist, then bail out and not load PV drivers.
>
> P.S.
> Ian Campbell suggested getting rid of 'xen_platform_pci_unplug'
> but unfortunatly the xen-blkfront driver is using it, so we
> cannot do it.
>
> Reported-by: Sander Eikelenboom Reported-by: Anthony PERARD
> Reported-by: Fabio Fantoni
For input:
Acked-by: Dmitry Torokhov
...
> +#if defined(CONFIG_XEN_PVHVM)
> +extern bool xen_has_pv_devices(void);
> +#else
> +static inline bool xen_has_pv_devices(void)
> +{
> +#if defined(CONFIG_XEN)
> + return true;
> +#else
> + return false;
> +#endif
return IS_ENABLED(CONFIG_XEN);
?
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On Wed, 2013-12-04 at 13:00 +, David Vrabel wrote: > On 03/12/13 21:14, Konrad Rzeszutek Wilk wrote: > > > > Ian Campbell suggested getting rid of 'xen_platform_pci_unplug' > > but unfortunatly the xen-blkfront driver is using it, so we > > cannot do it. > > I had a look at what blkfront was using this for and it seems dumb. How > did we end up with the frontend driver working around toolstack bugs? > If HVM Linux guest didn't want (e.g.,) PV CDROM, the toolstack shouldn't > have created one. Note that this cdrom stuff is actually nothing to do with the unplug variable -- it just happens to be right next to that check. Not that the use of the unplug var there doesn't also look pretty odd... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On 03/12/13 21:14, Konrad Rzeszutek Wilk wrote: > > Ian Campbell suggested getting rid of 'xen_platform_pci_unplug' > but unfortunatly the xen-blkfront driver is using it, so we > cannot do it. I had a look at what blkfront was using this for and it seems dumb. How did we end up with the frontend driver working around toolstack bugs? If HVM Linux guest didn't want (e.g.,) PV CDROM, the toolstack shouldn't have created one. And perhaps more importantly, if you actually want a PV CDROM in a HVM guest, it's not possible. David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On Wed, 2013-12-04 at 11:18 +, Stefano Stabellini wrote:
> On Wed, 4 Dec 2013, Ian Campbell wrote:
> > On Wed, 2013-12-04 at 11:05 +, Stefano Stabellini wrote:
> > > On Wed, 4 Dec 2013, Ian Campbell wrote:
> > > > On Wed, 2013-12-04 at 10:51 +, Stefano Stabellini wrote:
> > > > > On Wed, 4 Dec 2013, Ian Campbell wrote:
> > > > > > > +bool xen_has_pv_devices(void)
> > > > > > > +{
> > > > > > > + if (!xen_domain())
> > > > > > > + return false;
> > > > > > > +
> > > > > > > + if (xen_hvm_domain()) {
> > > > > > > + /* User requested no unplug, so no PV drivers. */
> > > > > > > + if (xen_emul_unplug & XEN_UNPLUG_NEVER)
> > > > > > > + return false;
> > > > > >
> > > > > > I think you need
> > > > > > if (xen_emul_unpug & XEN_UNPLUG_UNNECESSARY)
> > > > > > return true;
> > > > > > don't you?
> > > > >
> > > > > XEN_UNPLUG_UNNECESSARY was introduced to enable the platform PCI
> > > > > device
> > > > > even if it didn't respond properly to the unplug protocol.
> > > > > The corresponding parameter is called "unnecessary" because if you
> > > > > pass
> > > > > it to the kernel you mean that it is unnecessary to unplug the
> > > > > emulated
> > > > > devices but you can use the pv devices anyway.
> > > > >
> > > > > So no, we shouldn't check for XEN_UNPLUG_UNNECESSARY here.
> > > >
> > > > Oh, we will eventually fall through to the return true, so it does
> > > > actually work out OK.
> > > >
> > > > I'd still be in favour of handling each option explicitly, for clarity.
> > > > Which means checking for XEN_UNPLUG_UNNECESSARY.
> > >
> > > I think is wrong to check for any xen_emul_unpug options in this function.
> > > The xen_emul_unpug options should be used to set the right value of
> > > xen_platform_pci_unplug. (See my other reply.)
> >
> > Whichever one we check we should still be checking explicitly for the
> > "unnecessary" case, for clarity if nothing else.
>
> Sure, that is OK for me.
> In that case should we check for the full list of possible options?
We probably should. That probably means an extra
xen_has_pv_{disk,nic}_devices() which is the existing one plus the
specific checks?
>
> ide-disks -- unplug primary master IDE devices
> aux-ide-disks -- unplug non-primary-master IDE devices
> nics -- unplug network devices
> all -- unplug all emulated devices (NICs and IDE disks)
> unnecessary -- unplugging emulated devices is
> unnecessary even if the host did not respond to
> the unplug protocol
> never -- do not unplug even if version check succeeds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On Wed, 4 Dec 2013, Ian Campbell wrote:
> On Wed, 2013-12-04 at 11:05 +, Stefano Stabellini wrote:
> > On Wed, 4 Dec 2013, Ian Campbell wrote:
> > > On Wed, 2013-12-04 at 10:51 +, Stefano Stabellini wrote:
> > > > On Wed, 4 Dec 2013, Ian Campbell wrote:
> > > > > > +bool xen_has_pv_devices(void)
> > > > > > +{
> > > > > > + if (!xen_domain())
> > > > > > + return false;
> > > > > > +
> > > > > > + if (xen_hvm_domain()) {
> > > > > > + /* User requested no unplug, so no PV drivers. */
> > > > > > + if (xen_emul_unplug & XEN_UNPLUG_NEVER)
> > > > > > + return false;
> > > > >
> > > > > I think you need
> > > > > if (xen_emul_unpug & XEN_UNPLUG_UNNECESSARY)
> > > > > return true;
> > > > > don't you?
> > > >
> > > > XEN_UNPLUG_UNNECESSARY was introduced to enable the platform PCI device
> > > > even if it didn't respond properly to the unplug protocol.
> > > > The corresponding parameter is called "unnecessary" because if you pass
> > > > it to the kernel you mean that it is unnecessary to unplug the emulated
> > > > devices but you can use the pv devices anyway.
> > > >
> > > > So no, we shouldn't check for XEN_UNPLUG_UNNECESSARY here.
> > >
> > > Oh, we will eventually fall through to the return true, so it does
> > > actually work out OK.
> > >
> > > I'd still be in favour of handling each option explicitly, for clarity.
> > > Which means checking for XEN_UNPLUG_UNNECESSARY.
> >
> > I think is wrong to check for any xen_emul_unpug options in this function.
> > The xen_emul_unpug options should be used to set the right value of
> > xen_platform_pci_unplug. (See my other reply.)
>
> Whichever one we check we should still be checking explicitly for the
> "unnecessary" case, for clarity if nothing else.
Sure, that is OK for me.
In that case should we check for the full list of possible options?
ide-disks -- unplug primary master IDE devices
aux-ide-disks -- unplug non-primary-master IDE devices
nics -- unplug network devices
all -- unplug all emulated devices (NICs and IDE disks)
unnecessary -- unplugging emulated devices is
unnecessary even if the host did not respond to
the unplug protocol
never -- do not unplug even if version check succeeds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On Wed, 2013-12-04 at 11:05 +, Stefano Stabellini wrote:
> On Wed, 4 Dec 2013, Ian Campbell wrote:
> > On Wed, 2013-12-04 at 10:51 +, Stefano Stabellini wrote:
> > > On Wed, 4 Dec 2013, Ian Campbell wrote:
> > > > > +bool xen_has_pv_devices(void)
> > > > > +{
> > > > > + if (!xen_domain())
> > > > > + return false;
> > > > > +
> > > > > + if (xen_hvm_domain()) {
> > > > > + /* User requested no unplug, so no PV drivers. */
> > > > > + if (xen_emul_unplug & XEN_UNPLUG_NEVER)
> > > > > + return false;
> > > >
> > > > I think you need
> > > > if (xen_emul_unpug & XEN_UNPLUG_UNNECESSARY)
> > > > return true;
> > > > don't you?
> > >
> > > XEN_UNPLUG_UNNECESSARY was introduced to enable the platform PCI device
> > > even if it didn't respond properly to the unplug protocol.
> > > The corresponding parameter is called "unnecessary" because if you pass
> > > it to the kernel you mean that it is unnecessary to unplug the emulated
> > > devices but you can use the pv devices anyway.
> > >
> > > So no, we shouldn't check for XEN_UNPLUG_UNNECESSARY here.
> >
> > Oh, we will eventually fall through to the return true, so it does
> > actually work out OK.
> >
> > I'd still be in favour of handling each option explicitly, for clarity.
> > Which means checking for XEN_UNPLUG_UNNECESSARY.
>
> I think is wrong to check for any xen_emul_unpug options in this function.
> The xen_emul_unpug options should be used to set the right value of
> xen_platform_pci_unplug. (See my other reply.)
Whichever one we check we should still be checking explicitly for the
"unnecessary" case, for clarity if nothing else.
TBH I think the split between xen_emul_unplug and
xen_platform_pci_unplug is a bit artificial. There should be one value
which is static to platform-pci-unplug.c and accessor functions should
be provided for other code to use. Open coding all those accesses to
xen_platform_pci_unplug in every driver is just too error prone.
Ian.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On Wed, 4 Dec 2013, Ian Campbell wrote:
> On Wed, 2013-12-04 at 10:51 +, Stefano Stabellini wrote:
> > On Wed, 4 Dec 2013, Ian Campbell wrote:
> > > > +bool xen_has_pv_devices(void)
> > > > +{
> > > > + if (!xen_domain())
> > > > + return false;
> > > > +
> > > > + if (xen_hvm_domain()) {
> > > > + /* User requested no unplug, so no PV drivers. */
> > > > + if (xen_emul_unplug & XEN_UNPLUG_NEVER)
> > > > + return false;
> > >
> > > I think you need
> > > if (xen_emul_unpug & XEN_UNPLUG_UNNECESSARY)
> > > return true;
> > > don't you?
> >
> > XEN_UNPLUG_UNNECESSARY was introduced to enable the platform PCI device
> > even if it didn't respond properly to the unplug protocol.
> > The corresponding parameter is called "unnecessary" because if you pass
> > it to the kernel you mean that it is unnecessary to unplug the emulated
> > devices but you can use the pv devices anyway.
> >
> > So no, we shouldn't check for XEN_UNPLUG_UNNECESSARY here.
>
> Oh, we will eventually fall through to the return true, so it does
> actually work out OK.
>
> I'd still be in favour of handling each option explicitly, for clarity.
> Which means checking for XEN_UNPLUG_UNNECESSARY.
I think is wrong to check for any xen_emul_unpug options in this function.
The xen_emul_unpug options should be used to set the right value of
xen_platform_pci_unplug. (See my other reply.)
> > > > + /* And user has xen_platform_pci=0 set in guest config
> > > > as
> > > > +* driver did not modify the value. */
> > > > + if (!xen_platform_pci_unplug)
> > > > + return false;
>
> I assume this check doesn't trigger if unnecessary has been specified?
right
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On Wed, 2013-12-04 at 10:51 +, Stefano Stabellini wrote:
> On Wed, 4 Dec 2013, Ian Campbell wrote:
> > > +bool xen_has_pv_devices(void)
> > > +{
> > > + if (!xen_domain())
> > > + return false;
> > > +
> > > + if (xen_hvm_domain()) {
> > > + /* User requested no unplug, so no PV drivers. */
> > > + if (xen_emul_unplug & XEN_UNPLUG_NEVER)
> > > + return false;
> >
> > I think you need
> > if (xen_emul_unpug & XEN_UNPLUG_UNNECESSARY)
> > return true;
> > don't you?
>
> XEN_UNPLUG_UNNECESSARY was introduced to enable the platform PCI device
> even if it didn't respond properly to the unplug protocol.
> The corresponding parameter is called "unnecessary" because if you pass
> it to the kernel you mean that it is unnecessary to unplug the emulated
> devices but you can use the pv devices anyway.
>
> So no, we shouldn't check for XEN_UNPLUG_UNNECESSARY here.
Oh, we will eventually fall through to the return true, so it does
actually work out OK.
I'd still be in favour of handling each option explicitly, for clarity.
Which means checking for XEN_UNPLUG_UNNECESSARY.
> > > + /* And user has xen_platform_pci=0 set in guest config as
> > > + * driver did not modify the value. */
> > > + if (!xen_platform_pci_unplug)
> > > + return false;
I assume this check doesn't trigger if unnecessary has been specified?
> > > + }
> > > + return true;
> > > +}
> > > +EXPORT_SYMBOL_GPL(xen_has_pv_devices);
> > > +
> > > void xen_unplug_emulated_devices(void)
> > > {
> > > int r;
> >
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On Wed, 4 Dec 2013, Ian Campbell wrote:
> > +bool xen_has_pv_devices(void)
> > +{
> > + if (!xen_domain())
> > + return false;
> > +
> > + if (xen_hvm_domain()) {
> > + /* User requested no unplug, so no PV drivers. */
> > + if (xen_emul_unplug & XEN_UNPLUG_NEVER)
> > + return false;
>
> I think you need
> if (xen_emul_unpug & XEN_UNPLUG_UNNECESSARY)
> return true;
> don't you?
XEN_UNPLUG_UNNECESSARY was introduced to enable the platform PCI device
even if it didn't respond properly to the unplug protocol.
The corresponding parameter is called "unnecessary" because if you pass
it to the kernel you mean that it is unnecessary to unplug the emulated
devices but you can use the pv devices anyway.
So no, we shouldn't check for XEN_UNPLUG_UNNECESSARY here.
> > + /* And user has xen_platform_pci=0 set in guest config as
> > +* driver did not modify the value. */
> > + if (!xen_platform_pci_unplug)
> > + return false;
> > + }
> > + return true;
> > +}
> > +EXPORT_SYMBOL_GPL(xen_has_pv_devices);
> > +
> > void xen_unplug_emulated_devices(void)
> > {
> > int r;
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On Tue, 3 Dec 2013, Konrad Rzeszutek Wilk wrote:
> diff --git a/arch/x86/xen/platform-pci-unplug.c
> b/arch/x86/xen/platform-pci-unplug.c
> index 0a78524..087dfeb 100644
> --- a/arch/x86/xen/platform-pci-unplug.c
> +++ b/arch/x86/xen/platform-pci-unplug.c
> @@ -69,6 +69,24 @@ static int check_platform_magic(void)
> return 0;
> }
>
> +bool xen_has_pv_devices(void)
> +{
> + if (!xen_domain())
> + return false;
> +
> + if (xen_hvm_domain()) {
> + /* User requested no unplug, so no PV drivers. */
> + if (xen_emul_unplug & XEN_UNPLUG_NEVER)
> + return false;
Considering that if (xen_emul_unplug & XEN_UNPLUG_NEVER) we never set
xen_platform_pci_unplug, this check is redundant.
> + /* And user has xen_platform_pci=0 set in guest config as
> + * driver did not modify the value. */
> + if (!xen_platform_pci_unplug)
> + return false;
> + }
> + return true;
> +}
> +EXPORT_SYMBOL_GPL(xen_has_pv_devices);
> +
> void xen_unplug_emulated_devices(void)
> {
> int r;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On Tue, 2013-12-03 at 16:14 -0500, Konrad Rzeszutek Wilk wrote:
> The user has the option of disabling the platform driver:
> 00:02.0 Unassigned class [ff80]: XenSource, Inc. Xen Platform Device (rev 01)
>
> which is used to unplug the emulated drivers (IDE, Realtek 8169, etc)
> and allow the PV drivers to take over. If the user wishes
> to disable that they can set:
>
> xen_platform_pci=0
> (in the guest config file)
>
> or
> xen_emul_unplug=never
> (on the Linux command line)
>
> except it does not work properly. The PV drivers still try to
> load and since the Xen platform driver is not run - and it
> has not initialized the grant tables, most of the PV drivers
> stumble upon:
>
> input: Xen Virtual Keyboard as /devices/virtual/input/input5
> input: Xen Virtual Pointer as /devices/virtual/input/input6M
> [ cut here ]
> kernel BUG at /home/konrad/ssd/konrad/linux/drivers/xen/grant-table.c:1206!
> invalid opcode: [#1] SMP
> Modules linked in: xen_kbdfront(+) xenfs xen_privcmd
> CPU: 6 PID: 1389 Comm: modprobe Not tainted
> 3.13.0-rc1upstream-00021-ga6c892b-dirty #1
> Hardware name: Xen HVM domU, BIOS 4.4-unstable 11/26/2013
> RIP: 0010:[] []
> get_free_entries+0x2e0/0x300
> Call Trace:
> [] ? evdev_connect+0x1e3/0x240
> [] gnttab_grant_foreign_access+0x2e/0x70
> [] xenkbd_connect_backend+0x41/0x290 [xen_kbdfront]
> [] xenkbd_probe+0x2f2/0x324 [xen_kbdfront]
> [] xenbus_dev_probe+0x77/0x130
> [] xenbus_frontend_dev_probe+0x47/0x50
> [] driver_probe_device+0x89/0x230
> [] __driver_attach+0x9b/0xa0
> [] ? driver_probe_device+0x230/0x230
> [] ? driver_probe_device+0x230/0x230
> [] bus_for_each_dev+0x8c/0xb0
> [] driver_attach+0x19/0x20
> [] bus_add_driver+0x1a0/0x220
> [] driver_register+0x5f/0xf0
> [] xenbus_register_driver_common+0x15/0x20
> [] xenbus_register_frontend+0x23/0x40
> [] ? 0xa0014fff
> [] xenkbd_init+0x2b/0x1000 [xen_kbdfront]
> [] do_one_initcall+0x49/0x170
>
> .. snip..
>
> which is hardly nice. This patch fixes this by having each
> PV driver check for:
> - if running in PV, then it is fine to execute (as that is their
>native environment).
> - if running in HVM, check if user wanted 'xen_emul_unplug=never',
>in which case bail out and don't load PV drivers.
> - if running in HVM, and if PCI device 5853:0001 (xen_platform_pci)
>does not exist, then bail out and not load PV drivers.
>
> P.S.
> Ian Campbell suggested getting rid of 'xen_platform_pci_unplug'
> but unfortunatly the xen-blkfront driver is using it, so we
> cannot do it.
It might still be nice to expose a suitable semantic interface (i.e.
some relevant predicate) rather than the raw value for blkfront to use.
But that can be a future thing I think.
> Reported-by: Sander Eikelenboom Reported-by: Anthony PERARD
> Reported-by: Fabio Fantoni
> ---
> arch/x86/xen/platform-pci-unplug.c | 18 ++
> drivers/block/xen-blkfront.c | 2 +-
> drivers/char/tpm/xen-tpmfront.c| 4
> drivers/input/misc/xen-kbdfront.c | 4
> drivers/net/xen-netfront.c | 2 +-
> drivers/pci/xen-pcifront.c | 4
> drivers/video/xen-fbfront.c| 4
> drivers/xen/xenbus/xenbus_probe_frontend.c | 2 +-
> include/xen/platform_pci.h | 13 -
> 9 files changed, 49 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/xen/platform-pci-unplug.c
> b/arch/x86/xen/platform-pci-unplug.c
> index 0a78524..087dfeb 100644
> --- a/arch/x86/xen/platform-pci-unplug.c
> +++ b/arch/x86/xen/platform-pci-unplug.c
> @@ -69,6 +69,24 @@ static int check_platform_magic(void)
> return 0;
> }
>
> +bool xen_has_pv_devices(void)
> +{
> + if (!xen_domain())
> + return false;
> +
> + if (xen_hvm_domain()) {
> + /* User requested no unplug, so no PV drivers. */
> + if (xen_emul_unplug & XEN_UNPLUG_NEVER)
> + return false;
I think you need
if (xen_emul_unpug & XEN_UNPLUG_UNNECESSARY)
return true;
don't you?
> + /* And user has xen_platform_pci=0 set in guest config as
> + * driver did not modify the value. */
> + if (!xen_platform_pci_unplug)
> + return false;
> + }
> + return true;
> +}
> +EXPORT_SYMBOL_GPL(xen_has_pv_devices);
> +
> void xen_unplug_emulated_devices(void)
> {
> int r;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On Tue, 2013-12-03 at 16:14 -0500, Konrad Rzeszutek Wilk wrote:
The user has the option of disabling the platform driver:
00:02.0 Unassigned class [ff80]: XenSource, Inc. Xen Platform Device (rev 01)
which is used to unplug the emulated drivers (IDE, Realtek 8169, etc)
and allow the PV drivers to take over. If the user wishes
to disable that they can set:
xen_platform_pci=0
(in the guest config file)
or
xen_emul_unplug=never
(on the Linux command line)
except it does not work properly. The PV drivers still try to
load and since the Xen platform driver is not run - and it
has not initialized the grant tables, most of the PV drivers
stumble upon:
input: Xen Virtual Keyboard as /devices/virtual/input/input5
input: Xen Virtual Pointer as /devices/virtual/input/input6M
[ cut here ]
kernel BUG at /home/konrad/ssd/konrad/linux/drivers/xen/grant-table.c:1206!
invalid opcode: [#1] SMP
Modules linked in: xen_kbdfront(+) xenfs xen_privcmd
CPU: 6 PID: 1389 Comm: modprobe Not tainted
3.13.0-rc1upstream-00021-ga6c892b-dirty #1
Hardware name: Xen HVM domU, BIOS 4.4-unstable 11/26/2013
RIP: 0010:[813ddc40] [813ddc40]
get_free_entries+0x2e0/0x300
Call Trace:
[8150d9a3] ? evdev_connect+0x1e3/0x240
[813ddd0e] gnttab_grant_foreign_access+0x2e/0x70
[a0010081] xenkbd_connect_backend+0x41/0x290 [xen_kbdfront]
[a0010a12] xenkbd_probe+0x2f2/0x324 [xen_kbdfront]
[813e5757] xenbus_dev_probe+0x77/0x130
[813e7217] xenbus_frontend_dev_probe+0x47/0x50
[8145e9a9] driver_probe_device+0x89/0x230
[8145ebeb] __driver_attach+0x9b/0xa0
[8145eb50] ? driver_probe_device+0x230/0x230
[8145eb50] ? driver_probe_device+0x230/0x230
[8145cf1c] bus_for_each_dev+0x8c/0xb0
[8145e7d9] driver_attach+0x19/0x20
[8145e260] bus_add_driver+0x1a0/0x220
[8145f1ff] driver_register+0x5f/0xf0
[813e55c5] xenbus_register_driver_common+0x15/0x20
[813e76b3] xenbus_register_frontend+0x23/0x40
[a0015000] ? 0xa0014fff
[a001502b] xenkbd_init+0x2b/0x1000 [xen_kbdfront]
[81002049] do_one_initcall+0x49/0x170
.. snip..
which is hardly nice. This patch fixes this by having each
PV driver check for:
- if running in PV, then it is fine to execute (as that is their
native environment).
- if running in HVM, check if user wanted 'xen_emul_unplug=never',
in which case bail out and don't load PV drivers.
- if running in HVM, and if PCI device 5853:0001 (xen_platform_pci)
does not exist, then bail out and not load PV drivers.
P.S.
Ian Campbell suggested getting rid of 'xen_platform_pci_unplug'
but unfortunatly the xen-blkfront driver is using it, so we
cannot do it.
It might still be nice to expose a suitable semantic interface (i.e.
some relevant predicate) rather than the raw value for blkfront to use.
But that can be a future thing I think.
Reported-by: Sander Eikelenboom li...@eikelenboom.it
Reported-by: Anthony PERARD anthony.per...@citrix.com
Reported-by: Fabio Fantoni fabio.fant...@m2r.biz
---
arch/x86/xen/platform-pci-unplug.c | 18 ++
drivers/block/xen-blkfront.c | 2 +-
drivers/char/tpm/xen-tpmfront.c| 4
drivers/input/misc/xen-kbdfront.c | 4
drivers/net/xen-netfront.c | 2 +-
drivers/pci/xen-pcifront.c | 4
drivers/video/xen-fbfront.c| 4
drivers/xen/xenbus/xenbus_probe_frontend.c | 2 +-
include/xen/platform_pci.h | 13 -
9 files changed, 49 insertions(+), 4 deletions(-)
diff --git a/arch/x86/xen/platform-pci-unplug.c
b/arch/x86/xen/platform-pci-unplug.c
index 0a78524..087dfeb 100644
--- a/arch/x86/xen/platform-pci-unplug.c
+++ b/arch/x86/xen/platform-pci-unplug.c
@@ -69,6 +69,24 @@ static int check_platform_magic(void)
return 0;
}
+bool xen_has_pv_devices(void)
+{
+ if (!xen_domain())
+ return false;
+
+ if (xen_hvm_domain()) {
+ /* User requested no unplug, so no PV drivers. */
+ if (xen_emul_unplug XEN_UNPLUG_NEVER)
+ return false;
I think you need
if (xen_emul_unpug XEN_UNPLUG_UNNECESSARY)
return true;
don't you?
+ /* And user has xen_platform_pci=0 set in guest config as
+ * driver did not modify the value. */
+ if (!xen_platform_pci_unplug)
+ return false;
+ }
+ return true;
+}
+EXPORT_SYMBOL_GPL(xen_has_pv_devices);
+
void xen_unplug_emulated_devices(void)
{
int r;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On Tue, 3 Dec 2013, Konrad Rzeszutek Wilk wrote:
diff --git a/arch/x86/xen/platform-pci-unplug.c
b/arch/x86/xen/platform-pci-unplug.c
index 0a78524..087dfeb 100644
--- a/arch/x86/xen/platform-pci-unplug.c
+++ b/arch/x86/xen/platform-pci-unplug.c
@@ -69,6 +69,24 @@ static int check_platform_magic(void)
return 0;
}
+bool xen_has_pv_devices(void)
+{
+ if (!xen_domain())
+ return false;
+
+ if (xen_hvm_domain()) {
+ /* User requested no unplug, so no PV drivers. */
+ if (xen_emul_unplug XEN_UNPLUG_NEVER)
+ return false;
Considering that if (xen_emul_unplug XEN_UNPLUG_NEVER) we never set
xen_platform_pci_unplug, this check is redundant.
+ /* And user has xen_platform_pci=0 set in guest config as
+ * driver did not modify the value. */
+ if (!xen_platform_pci_unplug)
+ return false;
+ }
+ return true;
+}
+EXPORT_SYMBOL_GPL(xen_has_pv_devices);
+
void xen_unplug_emulated_devices(void)
{
int r;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On Wed, 4 Dec 2013, Ian Campbell wrote:
+bool xen_has_pv_devices(void)
+{
+ if (!xen_domain())
+ return false;
+
+ if (xen_hvm_domain()) {
+ /* User requested no unplug, so no PV drivers. */
+ if (xen_emul_unplug XEN_UNPLUG_NEVER)
+ return false;
I think you need
if (xen_emul_unpug XEN_UNPLUG_UNNECESSARY)
return true;
don't you?
XEN_UNPLUG_UNNECESSARY was introduced to enable the platform PCI device
even if it didn't respond properly to the unplug protocol.
The corresponding parameter is called unnecessary because if you pass
it to the kernel you mean that it is unnecessary to unplug the emulated
devices but you can use the pv devices anyway.
So no, we shouldn't check for XEN_UNPLUG_UNNECESSARY here.
+ /* And user has xen_platform_pci=0 set in guest config as
+* driver did not modify the value. */
+ if (!xen_platform_pci_unplug)
+ return false;
+ }
+ return true;
+}
+EXPORT_SYMBOL_GPL(xen_has_pv_devices);
+
void xen_unplug_emulated_devices(void)
{
int r;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On Wed, 2013-12-04 at 10:51 +, Stefano Stabellini wrote:
On Wed, 4 Dec 2013, Ian Campbell wrote:
+bool xen_has_pv_devices(void)
+{
+ if (!xen_domain())
+ return false;
+
+ if (xen_hvm_domain()) {
+ /* User requested no unplug, so no PV drivers. */
+ if (xen_emul_unplug XEN_UNPLUG_NEVER)
+ return false;
I think you need
if (xen_emul_unpug XEN_UNPLUG_UNNECESSARY)
return true;
don't you?
XEN_UNPLUG_UNNECESSARY was introduced to enable the platform PCI device
even if it didn't respond properly to the unplug protocol.
The corresponding parameter is called unnecessary because if you pass
it to the kernel you mean that it is unnecessary to unplug the emulated
devices but you can use the pv devices anyway.
So no, we shouldn't check for XEN_UNPLUG_UNNECESSARY here.
Oh, we will eventually fall through to the return true, so it does
actually work out OK.
I'd still be in favour of handling each option explicitly, for clarity.
Which means checking for XEN_UNPLUG_UNNECESSARY.
+ /* And user has xen_platform_pci=0 set in guest config as
+ * driver did not modify the value. */
+ if (!xen_platform_pci_unplug)
+ return false;
I assume this check doesn't trigger if unnecessary has been specified?
+ }
+ return true;
+}
+EXPORT_SYMBOL_GPL(xen_has_pv_devices);
+
void xen_unplug_emulated_devices(void)
{
int r;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On Wed, 4 Dec 2013, Ian Campbell wrote:
On Wed, 2013-12-04 at 10:51 +, Stefano Stabellini wrote:
On Wed, 4 Dec 2013, Ian Campbell wrote:
+bool xen_has_pv_devices(void)
+{
+ if (!xen_domain())
+ return false;
+
+ if (xen_hvm_domain()) {
+ /* User requested no unplug, so no PV drivers. */
+ if (xen_emul_unplug XEN_UNPLUG_NEVER)
+ return false;
I think you need
if (xen_emul_unpug XEN_UNPLUG_UNNECESSARY)
return true;
don't you?
XEN_UNPLUG_UNNECESSARY was introduced to enable the platform PCI device
even if it didn't respond properly to the unplug protocol.
The corresponding parameter is called unnecessary because if you pass
it to the kernel you mean that it is unnecessary to unplug the emulated
devices but you can use the pv devices anyway.
So no, we shouldn't check for XEN_UNPLUG_UNNECESSARY here.
Oh, we will eventually fall through to the return true, so it does
actually work out OK.
I'd still be in favour of handling each option explicitly, for clarity.
Which means checking for XEN_UNPLUG_UNNECESSARY.
I think is wrong to check for any xen_emul_unpug options in this function.
The xen_emul_unpug options should be used to set the right value of
xen_platform_pci_unplug. (See my other reply.)
+ /* And user has xen_platform_pci=0 set in guest config
as
+* driver did not modify the value. */
+ if (!xen_platform_pci_unplug)
+ return false;
I assume this check doesn't trigger if unnecessary has been specified?
right
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On Wed, 2013-12-04 at 11:05 +, Stefano Stabellini wrote:
On Wed, 4 Dec 2013, Ian Campbell wrote:
On Wed, 2013-12-04 at 10:51 +, Stefano Stabellini wrote:
On Wed, 4 Dec 2013, Ian Campbell wrote:
+bool xen_has_pv_devices(void)
+{
+ if (!xen_domain())
+ return false;
+
+ if (xen_hvm_domain()) {
+ /* User requested no unplug, so no PV drivers. */
+ if (xen_emul_unplug XEN_UNPLUG_NEVER)
+ return false;
I think you need
if (xen_emul_unpug XEN_UNPLUG_UNNECESSARY)
return true;
don't you?
XEN_UNPLUG_UNNECESSARY was introduced to enable the platform PCI device
even if it didn't respond properly to the unplug protocol.
The corresponding parameter is called unnecessary because if you pass
it to the kernel you mean that it is unnecessary to unplug the emulated
devices but you can use the pv devices anyway.
So no, we shouldn't check for XEN_UNPLUG_UNNECESSARY here.
Oh, we will eventually fall through to the return true, so it does
actually work out OK.
I'd still be in favour of handling each option explicitly, for clarity.
Which means checking for XEN_UNPLUG_UNNECESSARY.
I think is wrong to check for any xen_emul_unpug options in this function.
The xen_emul_unpug options should be used to set the right value of
xen_platform_pci_unplug. (See my other reply.)
Whichever one we check we should still be checking explicitly for the
unnecessary case, for clarity if nothing else.
TBH I think the split between xen_emul_unplug and
xen_platform_pci_unplug is a bit artificial. There should be one value
which is static to platform-pci-unplug.c and accessor functions should
be provided for other code to use. Open coding all those accesses to
xen_platform_pci_unplug in every driver is just too error prone.
Ian.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On Wed, 4 Dec 2013, Ian Campbell wrote:
On Wed, 2013-12-04 at 11:05 +, Stefano Stabellini wrote:
On Wed, 4 Dec 2013, Ian Campbell wrote:
On Wed, 2013-12-04 at 10:51 +, Stefano Stabellini wrote:
On Wed, 4 Dec 2013, Ian Campbell wrote:
+bool xen_has_pv_devices(void)
+{
+ if (!xen_domain())
+ return false;
+
+ if (xen_hvm_domain()) {
+ /* User requested no unplug, so no PV drivers. */
+ if (xen_emul_unplug XEN_UNPLUG_NEVER)
+ return false;
I think you need
if (xen_emul_unpug XEN_UNPLUG_UNNECESSARY)
return true;
don't you?
XEN_UNPLUG_UNNECESSARY was introduced to enable the platform PCI device
even if it didn't respond properly to the unplug protocol.
The corresponding parameter is called unnecessary because if you pass
it to the kernel you mean that it is unnecessary to unplug the emulated
devices but you can use the pv devices anyway.
So no, we shouldn't check for XEN_UNPLUG_UNNECESSARY here.
Oh, we will eventually fall through to the return true, so it does
actually work out OK.
I'd still be in favour of handling each option explicitly, for clarity.
Which means checking for XEN_UNPLUG_UNNECESSARY.
I think is wrong to check for any xen_emul_unpug options in this function.
The xen_emul_unpug options should be used to set the right value of
xen_platform_pci_unplug. (See my other reply.)
Whichever one we check we should still be checking explicitly for the
unnecessary case, for clarity if nothing else.
Sure, that is OK for me.
In that case should we check for the full list of possible options?
ide-disks -- unplug primary master IDE devices
aux-ide-disks -- unplug non-primary-master IDE devices
nics -- unplug network devices
all -- unplug all emulated devices (NICs and IDE disks)
unnecessary -- unplugging emulated devices is
unnecessary even if the host did not respond to
the unplug protocol
never -- do not unplug even if version check succeeds
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On Wed, 2013-12-04 at 11:18 +, Stefano Stabellini wrote:
On Wed, 4 Dec 2013, Ian Campbell wrote:
On Wed, 2013-12-04 at 11:05 +, Stefano Stabellini wrote:
On Wed, 4 Dec 2013, Ian Campbell wrote:
On Wed, 2013-12-04 at 10:51 +, Stefano Stabellini wrote:
On Wed, 4 Dec 2013, Ian Campbell wrote:
+bool xen_has_pv_devices(void)
+{
+ if (!xen_domain())
+ return false;
+
+ if (xen_hvm_domain()) {
+ /* User requested no unplug, so no PV drivers. */
+ if (xen_emul_unplug XEN_UNPLUG_NEVER)
+ return false;
I think you need
if (xen_emul_unpug XEN_UNPLUG_UNNECESSARY)
return true;
don't you?
XEN_UNPLUG_UNNECESSARY was introduced to enable the platform PCI
device
even if it didn't respond properly to the unplug protocol.
The corresponding parameter is called unnecessary because if you
pass
it to the kernel you mean that it is unnecessary to unplug the
emulated
devices but you can use the pv devices anyway.
So no, we shouldn't check for XEN_UNPLUG_UNNECESSARY here.
Oh, we will eventually fall through to the return true, so it does
actually work out OK.
I'd still be in favour of handling each option explicitly, for clarity.
Which means checking for XEN_UNPLUG_UNNECESSARY.
I think is wrong to check for any xen_emul_unpug options in this function.
The xen_emul_unpug options should be used to set the right value of
xen_platform_pci_unplug. (See my other reply.)
Whichever one we check we should still be checking explicitly for the
unnecessary case, for clarity if nothing else.
Sure, that is OK for me.
In that case should we check for the full list of possible options?
We probably should. That probably means an extra
xen_has_pv_{disk,nic}_devices() which is the existing one plus the
specific checks?
ide-disks -- unplug primary master IDE devices
aux-ide-disks -- unplug non-primary-master IDE devices
nics -- unplug network devices
all -- unplug all emulated devices (NICs and IDE disks)
unnecessary -- unplugging emulated devices is
unnecessary even if the host did not respond to
the unplug protocol
never -- do not unplug even if version check succeeds
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On 03/12/13 21:14, Konrad Rzeszutek Wilk wrote: Ian Campbell suggested getting rid of 'xen_platform_pci_unplug' but unfortunatly the xen-blkfront driver is using it, so we cannot do it. I had a look at what blkfront was using this for and it seems dumb. How did we end up with the frontend driver working around toolstack bugs? If HVM Linux guest didn't want (e.g.,) PV CDROM, the toolstack shouldn't have created one. And perhaps more importantly, if you actually want a PV CDROM in a HVM guest, it's not possible. David -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On Wed, 2013-12-04 at 13:00 +, David Vrabel wrote: On 03/12/13 21:14, Konrad Rzeszutek Wilk wrote: Ian Campbell suggested getting rid of 'xen_platform_pci_unplug' but unfortunatly the xen-blkfront driver is using it, so we cannot do it. I had a look at what blkfront was using this for and it seems dumb. How did we end up with the frontend driver working around toolstack bugs? If HVM Linux guest didn't want (e.g.,) PV CDROM, the toolstack shouldn't have created one. Note that this cdrom stuff is actually nothing to do with the unplug variable -- it just happens to be right next to that check. Not that the use of the unplug var there doesn't also look pretty odd... -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
Hi Konrad,
On Tue, Dec 03, 2013 at 04:14:06PM -0500, Konrad Rzeszutek Wilk wrote:
The user has the option of disabling the platform driver:
00:02.0 Unassigned class [ff80]: XenSource, Inc. Xen Platform Device (rev 01)
which is used to unplug the emulated drivers (IDE, Realtek 8169, etc)
and allow the PV drivers to take over. If the user wishes
to disable that they can set:
xen_platform_pci=0
(in the guest config file)
or
xen_emul_unplug=never
(on the Linux command line)
except it does not work properly. The PV drivers still try to
load and since the Xen platform driver is not run - and it
has not initialized the grant tables, most of the PV drivers
stumble upon:
input: Xen Virtual Keyboard as /devices/virtual/input/input5
input: Xen Virtual Pointer as /devices/virtual/input/input6M
[ cut here ]
kernel BUG at /home/konrad/ssd/konrad/linux/drivers/xen/grant-table.c:1206!
invalid opcode: [#1] SMP
Modules linked in: xen_kbdfront(+) xenfs xen_privcmd
CPU: 6 PID: 1389 Comm: modprobe Not tainted
3.13.0-rc1upstream-00021-ga6c892b-dirty #1
Hardware name: Xen HVM domU, BIOS 4.4-unstable 11/26/2013
RIP: 0010:[813ddc40] [813ddc40]
get_free_entries+0x2e0/0x300
Call Trace:
[8150d9a3] ? evdev_connect+0x1e3/0x240
[813ddd0e] gnttab_grant_foreign_access+0x2e/0x70
[a0010081] xenkbd_connect_backend+0x41/0x290 [xen_kbdfront]
[a0010a12] xenkbd_probe+0x2f2/0x324 [xen_kbdfront]
[813e5757] xenbus_dev_probe+0x77/0x130
[813e7217] xenbus_frontend_dev_probe+0x47/0x50
[8145e9a9] driver_probe_device+0x89/0x230
[8145ebeb] __driver_attach+0x9b/0xa0
[8145eb50] ? driver_probe_device+0x230/0x230
[8145eb50] ? driver_probe_device+0x230/0x230
[8145cf1c] bus_for_each_dev+0x8c/0xb0
[8145e7d9] driver_attach+0x19/0x20
[8145e260] bus_add_driver+0x1a0/0x220
[8145f1ff] driver_register+0x5f/0xf0
[813e55c5] xenbus_register_driver_common+0x15/0x20
[813e76b3] xenbus_register_frontend+0x23/0x40
[a0015000] ? 0xa0014fff
[a001502b] xenkbd_init+0x2b/0x1000 [xen_kbdfront]
[81002049] do_one_initcall+0x49/0x170
.. snip..
which is hardly nice. This patch fixes this by having each
PV driver check for:
- if running in PV, then it is fine to execute (as that is their
native environment).
- if running in HVM, check if user wanted 'xen_emul_unplug=never',
in which case bail out and don't load PV drivers.
- if running in HVM, and if PCI device 5853:0001 (xen_platform_pci)
does not exist, then bail out and not load PV drivers.
P.S.
Ian Campbell suggested getting rid of 'xen_platform_pci_unplug'
but unfortunatly the xen-blkfront driver is using it, so we
cannot do it.
Reported-by: Sander Eikelenboom li...@eikelenboom.it
Reported-by: Anthony PERARD anthony.per...@citrix.com
Reported-by: Fabio Fantoni fabio.fant...@m2r.biz
For input:
Acked-by: Dmitry Torokhov dmitry.torok...@gmail.com
...
+#if defined(CONFIG_XEN_PVHVM)
+extern bool xen_has_pv_devices(void);
+#else
+static inline bool xen_has_pv_devices(void)
+{
+#if defined(CONFIG_XEN)
+ return true;
+#else
+ return false;
+#endif
return IS_ENABLED(CONFIG_XEN);
?
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
which is hardly nice. This patch fixes this by having each
PV driver check for:
- if running in PV, then it is fine to execute (as that is their
native environment).
- if running in HVM, check if user wanted 'xen_emul_unplug=never',
in which case bail out and don't load PV drivers.
- if running in HVM, and if PCI device 5853:0001 (xen_platform_pci)
does not exist, then bail out and not load PV drivers.
P.S.
Ian Campbell suggested getting rid of 'xen_platform_pci_unplug'
but unfortunatly the xen-blkfront driver is using it, so we
cannot do it.
Reported-by: Sander Eikelenboom li...@eikelenboom.it
Reported-by: Anthony PERARD anthony.per...@citrix.com
Reported-by: Fabio Fantoni fabio.fant...@m2r.biz
For input:
Acked-by: Dmitry Torokhov dmitry.torok...@gmail.com
Thank you. I need to do some extra changes to the other subsystems but
for the fb/kbd itshould be still throught the same function as this
patch has exposed. I will repost it and include your Ack if you are OK
with that?
...
+#if defined(CONFIG_XEN_PVHVM)
+extern bool xen_has_pv_devices(void);
+#else
+static inline bool xen_has_pv_devices(void)
+{
+#if defined(CONFIG_XEN)
+ return true;
+#else
+ return false;
+#endif
return IS_ENABLED(CONFIG_XEN);
?
Oh, awesome! Thanks!
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
On Wed, Dec 04, 2013 at 11:48:03AM -0500, Konrad Rzeszutek Wilk wrote: which is hardly nice. This patch fixes this by having each PV driver check for: - if running in PV, then it is fine to execute (as that is their native environment). - if running in HVM, check if user wanted 'xen_emul_unplug=never', in which case bail out and don't load PV drivers. - if running in HVM, and if PCI device 5853:0001 (xen_platform_pci) does not exist, then bail out and not load PV drivers. P.S. Ian Campbell suggested getting rid of 'xen_platform_pci_unplug' but unfortunatly the xen-blkfront driver is using it, so we cannot do it. Reported-by: Sander Eikelenboom li...@eikelenboom.it Reported-by: Anthony PERARD anthony.per...@citrix.com Reported-by: Fabio Fantoni fabio.fant...@m2r.biz For input: Acked-by: Dmitry Torokhov dmitry.torok...@gmail.com Thank you. I need to do some extra changes to the other subsystems but for the fb/kbd itshould be still throught the same function as this patch has exposed. I will repost it and include your Ack if you are OK with that? Sure. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
