Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On 14/05/2015 14:02, Markus Armbruster wrote: It should certainly be off for pc-q35-2.4 and newer. Real Q35 boards commonly don't have an FDC (depends on the Super I/O chip used). We may want to keep it off for pc-i440fx-2.4 and newer. I doubt there's a real i440FX without an FDC, but our virtual i440FX is quite unlike a real one in other ways already. That would break libvirt for people upgrading from 2.3 to 2.4. So it's more like pc-i440fx-3.0 and pc-q35-3.0. Unless for q35 we decide to break everything and retroactively nuke the controller. (I'm still not sure why we have backwards-compatible machine types for q35). Paolo * Create the FDC only if the option is on. * Optional: make -drive if=floppy,... auto-enable it I wouldn't bother doing the same for -global isa-fdc.driveA=... and such. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On 14/05/2015 14:45, Markus Armbruster wrote: Paolo Bonzini pbonz...@redhat.com writes: On 14/05/2015 14:02, Markus Armbruster wrote: It should certainly be off for pc-q35-2.4 and newer. Real Q35 boards commonly don't have an FDC (depends on the Super I/O chip used). We may want to keep it off for pc-i440fx-2.4 and newer. I doubt there's a real i440FX without an FDC, but our virtual i440FX is quite unlike a real one in other ways already. That would break libvirt for people upgrading from 2.3 to 2.4. So it's more like pc-i440fx-3.0 and pc-q35-3.0. What exactly breaks when? libvirt expects -nodefaults -drive if=none,id=fdd0,... -global isa-fdc.driveA=fdd0 to result in a machine with a working FDD. It doesn't know that it has to add -machine fdc=on. Besides, adding a new machine option is not the best we can do. If the default is no FDC, all that is needed to add one back is -device. An FDC is yet another ISA device, it is possible to create one with -device. add the magic to make -global isa-fdc... auto-set the option to on. That would be ugly magic. The more I think about this, the more I think this is just a kneejerk reaction to a sensationalist announcement. The effect of this vulnerability on properly configured data centers (running non-prehistoric versions of Xen or KVM and using stubdom/SELinux/AppArmor properly) should be really close to zero. It's a storm in a tea cup. Paolo Unless for q35 we decide to break everything and retroactively nuke the controller. (I'm still not sure why we have backwards-compatible machine types for q35). Beats me :) [...] ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
Thursday, May 14, 2015, 2:53:17 PM, you wrote: On 14/05/2015 14:45, Markus Armbruster wrote: Paolo Bonzini pbonz...@redhat.com writes: On 14/05/2015 14:02, Markus Armbruster wrote: It should certainly be off for pc-q35-2.4 and newer. Real Q35 boards commonly don't have an FDC (depends on the Super I/O chip used). We may want to keep it off for pc-i440fx-2.4 and newer. I doubt there's a real i440FX without an FDC, but our virtual i440FX is quite unlike a real one in other ways already. That would break libvirt for people upgrading from 2.3 to 2.4. So it's more like pc-i440fx-3.0 and pc-q35-3.0. What exactly breaks when? libvirt expects -nodefaults -drive if=none,id=fdd0,... -global isa-fdc.driveA=fdd0 to result in a machine with a working FDD. It doesn't know that it has to add -machine fdc=on. Besides, adding a new machine option is not the best we can do. If the default is no FDC, all that is needed to add one back is -device. An FDC is yet another ISA device, it is possible to create one with -device. add the magic to make -global isa-fdc... auto-set the option to on. That would be ugly magic. The more I think about this, the more I think this is just a kneejerk reaction to a sensationalist announcement. The effect of this vulnerability on properly configured data centers (running non-prehistoric versions of Xen or KVM and using stubdom/SELinux/AppArmor properly) should be really close to zero. It's a storm in a tea cup. Paolo I tend to kindly disagree if you look at the broader perspective, yes it's could be a storm in a tea cup, but there seems to be a pattern. From a cmdline user / platform emulation point of view i can imagine that some sort of auto configuration / bundling in platforms is necessary (to limit the length of the cmdline to be supplied). But from a library / toolstack point of view it's quite nasty if *all* more or less obscure options have to actively disabled. It's quite undoable, not to mention what happens when new ones get added. From this PoV it would be better to have to actively enable all the stuff you want. At least Xen seemed to have taken the no-defaults as the best option to get there so far, but that doesn't seem to It's not the first CVE that has come from this you have to actively disable all you don't want to happen and probably won't be the last. So a -no-defaults-now-for-real option/mode for libraries / toolstacks, that requires them to be very verbose, explicit and specific in what they actually want, could be valuable. -- Sander Unless for q35 we decide to break everything and retroactively nuke the controller. (I'm still not sure why we have backwards-compatible machine types for q35). Beats me :) [...] ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On 14/05/2015 15:25, Sander Eikelenboom wrote: I tend to kindly disagree if you look at the broader perspective, yes it's could be a storm in a tea cup, but there seems to be a pattern. From a cmdline user / platform emulation point of view i can imagine that some sort of auto configuration / bundling in platforms is necessary (to limit the length of the cmdline to be supplied). But from a library / toolstack point of view it's quite nasty if *all* more or less obscure options have to actively disabled. It's quite undoable, not to mention what happens when new ones get added. Where do you stop? Do you want to disable ACPI by default? It's a relatively large amount of code, but for most modern guests it is necessary. Besides, removing just the floppy drive makes little sense. The following devices remain with -nodefaults: - an HPET - the power management device, which includes an I2C controller - an IDE controller - a keyboard controller - the magic VMware I/O port - the PC speaker (!) - the legacy PIT - the real-time clock - the two PICs and IOAPIC Of all these, most guests will only use the PM device (maybe) and a small subset of the RTC. At the very least, the IDE controller, PC speaker and the HPET should be removed by -nodefaults-i-mean-it. If you're using UEFI firmware, probably you could remove everything except the PM device---maybe the RTC and the IOAPIC. At this point this is not -nodefaults-i-mean-it, it's a different machine type altogether and it's remarkably different from a PC. Reducing the attack surface is always nice, but hypervisor and device model bugs are going to exist forever. The amount of code for legacy devices is all in all not that big, and I can hardly recall other vulnerabilities in there. This is why I say it's a knee-jerk reaction. It is the intrinsic problem of virtualization mentioned in the famous quote of Theo de Raadt(*). Even if you do PV like Xen or Hyper-V, you have less or no QEMU code but you throw more hypervisor code in untrusted hands (hypervisor bugs exist, e.g. CVE-2012-4539, CVE-2012-5510 or CVE-2013-1964). (*) Which of course misses the point of virtualization altogether. Once you have established that you need more density, choosing containers vs. virtualization is a gamble on whether you prefer more moving parts and more layers that have to be broken (more isolation), or fewer moving parts and less isolation. Paolo From this PoV it would be better to have to actively enable all the stuff you want. At least Xen seemed to have taken the no-defaults as the best option to get there so far, but that doesn't seem to It's not the first CVE that has come from this you have to actively disable all you don't want to happen and probably won't be the last. So a -no-defaults-now-for-real option/mode for libraries / toolstacks, that requires them to be very verbose, explicit and specific in what they actually want, could be valuable. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On Thu, 14 May 2015, Paolo Bonzini wrote: On 14/05/2015 15:25, Sander Eikelenboom wrote: I tend to kindly disagree if you look at the broader perspective, yes it's could be a storm in a tea cup, but there seems to be a pattern. From a cmdline user / platform emulation point of view i can imagine that some sort of auto configuration / bundling in platforms is necessary (to limit the length of the cmdline to be supplied). But from a library / toolstack point of view it's quite nasty if *all* more or less obscure options have to actively disabled. It's quite undoable, not to mention what happens when new ones get added. Where do you stop? At this stage I would be happy enough if no floppy drives were actually emulated when the user asks for none. Do you want to disable ACPI by default? It's a relatively large amount of code, but for most modern guests it is necessary. Besides, removing just the floppy drive makes little sense. The following devices remain with -nodefaults: - an HPET - the power management device, which includes an I2C controller - an IDE controller - a keyboard controller - the magic VMware I/O port - the PC speaker (!) - the legacy PIT - the real-time clock - the two PICs and IOAPIC Of all these, most guests will only use the PM device (maybe) and a small subset of the RTC. At the very least, the IDE controller, PC speaker and the HPET should be removed by -nodefaults-i-mean-it. If you're using UEFI firmware, probably you could remove everything except the PM device---maybe the RTC and the IOAPIC. At this point this is not -nodefaults-i-mean-it, it's a different machine type altogether and it's remarkably different from a PC. Reducing the attack surface is always nice, but hypervisor and device model bugs are going to exist forever. The amount of code for legacy devices is all in all not that big, and I can hardly recall other vulnerabilities in there. This is why I say it's a knee-jerk reaction. It is the intrinsic problem of virtualization mentioned in the famous quote of Theo de Raadt(*). Even if you do PV like Xen or Hyper-V, you have less or no QEMU code but you throw more hypervisor code in untrusted hands (hypervisor bugs exist, e.g. CVE-2012-4539, CVE-2012-5510 or CVE-2013-1964). (*) Which of course misses the point of virtualization altogether. Once you have established that you need more density, choosing containers vs. virtualization is a gamble on whether you prefer more moving parts and more layers that have to be broken (more isolation), or fewer moving parts and less isolation. Sure, but it is harder to write a device emulator in a secure fashion than a brand new PV interface, that can be designed with security in mind from scratch. The number of VM escaping CVEs affecting Xen PV interfaces is extremely low, I think it was just two since the start of the project. From this PoV it would be better to have to actively enable all the stuff you want. At least Xen seemed to have taken the no-defaults as the best option to get there so far, but that doesn't seem to It's not the first CVE that has come from this you have to actively disable all you don't want to happen and probably won't be the last. So a -no-defaults-now-for-real option/mode for libraries / toolstacks, that requires them to be very verbose, explicit and specific in what they actually want, could be valuable. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On Thu, May 14, 2015 at 02:02:04PM +0200, Markus Armbruster wrote: Correct. Here's how I think it should be done: * Create a machine option to control the FDC This is a machine-specific option. It should only exist for machine types that have an optional FDC. Default must be on for old machine types. Default may be off for new machine types. It should certainly be off for pc-q35-2.4 and newer. Real Q35 boards commonly don't have an FDC (depends on the Super I/O chip used). We may want to keep it off for pc-i440fx-2.4 and newer. I doubt there's a real i440FX without an FDC, but our virtual i440FX is quite unlike a real one in other ways already. I think making it off by default is a bad idea, it will break command-line users. * Create the FDC only if the option is on. * Optional: make -drive if=floppy,... auto-enable it Every time we do such auto hacks, we regret this later. Just do what we are told, fail if=floppy if disabled. I wouldn't bother doing the same for -global isa-fdc.driveA=... and such. Stefano, if you're willing to tackle this, go right ahead! ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On Thu, May 14, 2015 at 02:45:30PM +0200, Markus Armbruster wrote: Paolo Bonzini pbonz...@redhat.com writes: On 14/05/2015 14:02, Markus Armbruster wrote: It should certainly be off for pc-q35-2.4 and newer. Real Q35 boards commonly don't have an FDC (depends on the Super I/O chip used). We may want to keep it off for pc-i440fx-2.4 and newer. I doubt there's a real i440FX without an FDC, but our virtual i440FX is quite unlike a real one in other ways already. That would break libvirt for people upgrading from 2.3 to 2.4. So it's more like pc-i440fx-3.0 and pc-q35-3.0. What exactly breaks when? [quote] * Create the FDC only if the option is on. * Optional: make -drive if=floppy,... auto-enable it I wouldn't bother doing the same for -global isa-fdc.driveA=... and such. [/quote] Libvirt uses -global when enabling floppy devices. So since current libvirt does not know about the new (to be created) machine type property to turn on FDC, it will get an error using -global isa-fdc.driveA= I'm not too bothered about this, as long as libvirt has enough advance notice to add support for the new machine type property to enable FDC before we change its default value to be off. Perhaps one QEMU major release cycle before toggling the default, to give time for new libvirt to penetrate to distros Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On Thu, May 14, 2015 at 03:25:39PM +0200, Sander Eikelenboom wrote: Thursday, May 14, 2015, 2:53:17 PM, you wrote: On 14/05/2015 14:45, Markus Armbruster wrote: Paolo Bonzini pbonz...@redhat.com writes: On 14/05/2015 14:02, Markus Armbruster wrote: It should certainly be off for pc-q35-2.4 and newer. Real Q35 boards commonly don't have an FDC (depends on the Super I/O chip used). We may want to keep it off for pc-i440fx-2.4 and newer. I doubt there's a real i440FX without an FDC, but our virtual i440FX is quite unlike a real one in other ways already. That would break libvirt for people upgrading from 2.3 to 2.4. So it's more like pc-i440fx-3.0 and pc-q35-3.0. What exactly breaks when? libvirt expects -nodefaults -drive if=none,id=fdd0,... -global isa-fdc.driveA=fdd0 to result in a machine with a working FDD. It doesn't know that it has to add -machine fdc=on. Besides, adding a new machine option is not the best we can do. If the default is no FDC, all that is needed to add one back is -device. An FDC is yet another ISA device, it is possible to create one with -device. add the magic to make -global isa-fdc... auto-set the option to on. That would be ugly magic. The more I think about this, the more I think this is just a kneejerk reaction to a sensationalist announcement. The effect of this vulnerability on properly configured data centers (running non-prehistoric versions of Xen or KVM and using stubdom/SELinux/AppArmor properly) should be really close to zero. It's a storm in a tea cup. Paolo I agree. An option to disable fdc sounds like a reasonable thing to do reduce attack surface while keeping full compatibility. After all downstreams disable devices they don't want to support, too. Changing defaults just because a device had a bug does sound extreme. I tend to kindly disagree if you look at the broader perspective, yes it's could be a storm in a tea cup, but there seems to be a pattern. From a cmdline user / platform emulation point of view i can imagine that some sort of auto configuration / bundling in platforms is necessary (to limit the length of the cmdline to be supplied). But from a library / toolstack point of view it's quite nasty if *all* more or less obscure options have to actively disabled. It's quite undoable, not to mention what happens when new ones get added. I support this statement for new features. Users should opt-in to them. E.g. we learned this the hard way with pvpanic. From this PoV it would be better to have to actively enable all the stuff you want. At least Xen seemed to have taken the no-defaults as the best option to get there so far, but that doesn't seem to It's not the first CVE that has come from this you have to actively disable all you don't want to happen and probably won't be the last. So a -no-defaults-now-for-real option/mode for libraries / toolstacks, that requires them to be very verbose, explicit and specific in what they actually want, could be valuable. -- Sander If you would change what this flag means in each release, that would achieve no useful purpose IMO. Unless for q35 we decide to break everything and retroactively nuke the controller. (I'm still not sure why we have backwards-compatible machine types for q35). Beats me :) [...] I'm fine with making them all identical. -- MST ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On 14/05/2015 16:39, Stefano Stabellini wrote: On Thu, 14 May 2015, Paolo Bonzini wrote: On 14/05/2015 15:25, Sander Eikelenboom wrote: I tend to kindly disagree if you look at the broader perspective, yes it's could be a storm in a tea cup, but there seems to be a pattern. From a cmdline user / platform emulation point of view i can imagine that some sort of auto configuration / bundling in platforms is necessary (to limit the length of the cmdline to be supplied). But from a library / toolstack point of view it's quite nasty if *all* more or less obscure options have to actively disabled. It's quite undoable, not to mention what happens when new ones get added. Where do you stop? At this stage I would be happy enough if no floppy drives were actually emulated when the user asks for none. Floppy drives aren't being emulated; the controller is. Same for IDE, so why single out the FDD controller? Sure, but it is harder to write a device emulator in a secure fashion than a brand new PV interface, that can be designed with security in mind from scratch. The number of VM escaping CVEs affecting Xen PV interfaces is extremely low, I think it was just two since the start of the project. Sure; OTOH, treating hypervisor and QEMU escapes would be very silly, if QEMU has been properly protected (through any combination of stubdoms, LSM, seccomp, ...). Paolo From this PoV it would be better to have to actively enable all the stuff you want. At least Xen seemed to have taken the no-defaults as the best option to get there so far, but that doesn't seem to It's not the first CVE that has come from this you have to actively disable all you don't want to happen and probably won't be the last. So a -no-defaults-now-for-real option/mode for libraries / toolstacks, that requires them to be very verbose, explicit and specific in what they actually want, could be valuable. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
Daniel P. Berrange berra...@redhat.com writes: On Thu, May 14, 2015 at 12:12:52PM +0100, Stefano Stabellini wrote: On Wed, 13 May 2015, John Snow wrote: On 05/13/2015 02:15 PM, Stefano Stabellini wrote: On Wed, 13 May 2015, Daniel P. Berrange wrote: On Wed, May 13, 2015 at 06:29:46PM +0100, Stefano Stabellini wrote: Do not emulate a floppy drive if no drives are supposed to be present. This fixes the behavior of -nodefaults, that should remove the floppy drive (see docs/qdev-device-use.txt:Default Devices), but actually doesn't. Technically that doc is just refering to the disablement of the primary floppy drive, as opposed to the floppy controller. The floppy controller itself is really a built-in device that is defined as part of the machine type, along with the various other platform devices hanging off the PIIX and ISA brige. I think you are right, this patch is a bit too harsh in fixing the problem: I just wanted to properly disable drive emulation, because from my tests the guest thinks that one drive is present even when is not. We should just be a little careful in explaining the difference between the controller, the drives, and what circumstances things show up and to whom. Currently the FDC is always present and always has two drive objects that are directly inlined to that device. Now, based on whether or not this line fires in vl.c: default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS); controls whether or not we find that drive in pc_basic_device_init as you've found, which controls (ultimately) whether or not fdctrl-drives[0].blk or fdctrl-drives[1].blk gets set. If the blk pointer is set, even if you have no media inserted, fd_revalidate (and pick_geometry) will *FORCIBLY PICK* a drive type for this floppy, currently a 1.44MB type, (20 sect, 80 track, 1 head) with a data rate of 500 Kbps. This is written to the rtc memory where seabios later reads it to discover if you have a guest-visible floppy drive there. Both QEMU and SeaBIOS confuse the concept of A drive is present with A disk is present which leads to these kinds of confusing scenarios. There's some work to do here, for sure. That was my impression too. On Thu, 14 May 2015, Stefan Weil wrote: Am 13.05.2015 um 20:15 schrieb Stefano Stabellini: On Wed, 13 May 2015, Daniel P. Berrange wrote: On Wed, May 13, 2015 at 06:29:46PM +0100, Stefano Stabellini wrote: Do not emulate a floppy drive if no drives are supposed to be present. This fixes the behavior of -nodefaults, that should remove the floppy drive (see docs/qdev-device-use.txt:Default Devices), but actually doesn't. Technically that doc is just refering to the disablement of the primary floppy drive, as opposed to the floppy controller. The floppy controller itself is really a built-in device that is defined as part of the machine type, along with the various other platform devices hanging off the PIIX and ISA brige. I think you are right, this patch is a bit too harsh in fixing the problem: I just wanted to properly disable drive emulation, because from my tests the guest thinks that one drive is present even when is not. A short test on some of my physical machines shows that most of them don't have a floppy disk controller at all (dmesg | grep FDC). Only some older machines still have one. Therefore I think that QEMU must also be able to offer a virtual machine without an FDC, maybe as the default for the next version of QEMU. I think it would make sense to make it the default for -nodefaults machines. I would be OK with a new property too, as we could set it from libxl or libvirt. Anybody would be happy to pick this one up or should I do it? It isn't permissible to change the hardware exposed to guests for existing machine types, even when -nodefaults is used. Any change in defaults has to be for new machine types only. eg we can't change pc-2.3.0 machine type, but we could change defaults for the pc-2.4.0 machine type that will be in next release. That ensures migration upgrade compatibility for existing guests, while letting new guests get the new defaults. Correct. Here's how I think it should be done: * Create a machine option to control the FDC This is a machine-specific option. It should only exist for machine types that have an optional FDC. Default must be on for old machine types. Default may be off for new machine types. It should certainly be off for pc-q35-2.4 and newer. Real Q35 boards commonly don't have an FDC (depends on the Super I/O chip used). We may want to keep it off for pc-i440fx-2.4 and newer. I doubt there's a real i440FX without an FDC, but our virtual i440FX is quite unlike a real one in other ways already. * Create the FDC only if the option is on. * Optional: make -drive
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
Paolo Bonzini pbonz...@redhat.com writes: On 14/05/2015 13:47, Michael S. Tsirkin wrote: I would be OK with a new property too, as we could set it from libxl or libvirt. Anybody would be happy to pick this one up or should I do it? Pls go ahead, I can merge it in the pc tree. Note that because of the -drive if=none,id=fdd1 -global isa-fdc.driveA=fdd1 trick to create a floppy drive without if=floppy, the default should remain on---at least for a few releases---even if -nodefaults is passed. Unless we bite the bullet and add the magic to make -global isa-fdc... auto-set the option to on. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
Paolo Bonzini pbonz...@redhat.com writes: On 14/05/2015 14:02, Markus Armbruster wrote: It should certainly be off for pc-q35-2.4 and newer. Real Q35 boards commonly don't have an FDC (depends on the Super I/O chip used). We may want to keep it off for pc-i440fx-2.4 and newer. I doubt there's a real i440FX without an FDC, but our virtual i440FX is quite unlike a real one in other ways already. That would break libvirt for people upgrading from 2.3 to 2.4. So it's more like pc-i440fx-3.0 and pc-q35-3.0. What exactly breaks when? Unless for q35 we decide to break everything and retroactively nuke the controller. (I'm still not sure why we have backwards-compatible machine types for q35). Beats me :) [...] ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On 14/05/2015 13:47, Michael S. Tsirkin wrote: I would be OK with a new property too, as we could set it from libxl or libvirt. Anybody would be happy to pick this one up or should I do it? Pls go ahead, I can merge it in the pc tree. Note that because of the -drive if=none,id=fdd1 -global isa-fdc.driveA=fdd1 trick to create a floppy drive without if=floppy, the default should remain on---at least for a few releases---even if -nodefaults is passed. Paolo ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On Thu, May 14, 2015 at 01:54:22PM +0200, Paolo Bonzini wrote: On 14/05/2015 13:47, Michael S. Tsirkin wrote: I would be OK with a new property too, as we could set it from libxl or libvirt. Anybody would be happy to pick this one up or should I do it? Pls go ahead, I can merge it in the pc tree. Note that because of the -drive if=none,id=fdd1 -global isa-fdc.driveA=fdd1 trick to create a floppy drive without if=floppy, the default should remain on---at least for a few releases---even if -nodefaults is passed. Paolo Exactly. -- MST ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On Thu, 14 May 2015, Paolo Bonzini wrote: On 14/05/2015 16:39, Stefano Stabellini wrote: On Thu, 14 May 2015, Paolo Bonzini wrote: On 14/05/2015 15:25, Sander Eikelenboom wrote: I tend to kindly disagree if you look at the broader perspective, yes it's could be a storm in a tea cup, but there seems to be a pattern. From a cmdline user / platform emulation point of view i can imagine that some sort of auto configuration / bundling in platforms is necessary (to limit the length of the cmdline to be supplied). But from a library / toolstack point of view it's quite nasty if *all* more or less obscure options have to actively disabled. It's quite undoable, not to mention what happens when new ones get added. Where do you stop? At this stage I would be happy enough if no floppy drives were actually emulated when the user asks for none. Floppy drives aren't being emulated; the controller is. Same for IDE, so why single out the FDD controller? The problem is that floppy drive emulation code in the controller is not completely turned off even when no drives are available, see: http://marc.info/?l=xen-develm=143155839722714w=2 Instead of disentangling the controller code, I am taking the easy route of removing the controller. Sure, but it is harder to write a device emulator in a secure fashion than a brand new PV interface, that can be designed with security in mind from scratch. The number of VM escaping CVEs affecting Xen PV interfaces is extremely low, I think it was just two since the start of the project. Sure; OTOH, treating hypervisor and QEMU escapes would be very silly, if QEMU has been properly protected (through any combination of stubdoms, LSM, seccomp, ...). That is true, but I don't know how many distros setup up selinux, stubdoms, etc properly by default, but I am guessing not that many. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On 05/14/2015 10:07 AM, Michael S. Tsirkin wrote: On Thu, May 14, 2015 at 02:02:04PM +0200, Markus Armbruster wrote: Correct. Here's how I think it should be done: * Create a machine option to control the FDC This is a machine-specific option. It should only exist for machine types that have an optional FDC. Default must be on for old machine types. Default may be off for new machine types. It should certainly be off for pc-q35-2.4 and newer. Real Q35 boards commonly don't have an FDC (depends on the Super I/O chip used). We may want to keep it off for pc-i440fx-2.4 and newer. I doubt there's a real i440FX without an FDC, but our virtual i440FX is quite unlike a real one in other ways already. I think making it off by default is a bad idea, it will break command-line users. If we can add a flag to disable it, I still think I wouldn't mind that, if it could be worked out to not be hacky and gross. * Create the FDC only if the option is on. * Optional: make -drive if=floppy,... auto-enable it Every time we do such auto hacks, we regret this later. Just do what we are told, fail if=floppy if disabled. I agree very much. Just because the current drive/device syntax is almost totally hosed doesn't mean we should put more wood on the fire. I wouldn't bother doing the same for -global isa-fdc.driveA=... and such. Stefano, if you're willing to tackle this, go right ahead! I'm definitely against a --seriously-nothing flag because the line for what is embedded or not is fuzzy. Paolo raises some good points against where you draw the line for what we decide to allow users to include/exclude that is otherwise considered part of the board. Still, given the hype train, if there is an API we could introduce that is likely not to make our code gross (or make us belly-ache about how dumb we were in 5 years) that disables the FDC, I don't think I would mind terribly. I'll leave that to minds more opinionated than mine to hash out, though. Maybe the best option here really is to carefully separate optional from non-optional components (FDC vs. Floppy Drive, Floppy Disk code) and just give the core FDC code a good scrubbing. --js ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On Wed, 13 May 2015, John Snow wrote: On 05/13/2015 02:15 PM, Stefano Stabellini wrote: On Wed, 13 May 2015, Daniel P. Berrange wrote: On Wed, May 13, 2015 at 06:29:46PM +0100, Stefano Stabellini wrote: Do not emulate a floppy drive if no drives are supposed to be present. This fixes the behavior of -nodefaults, that should remove the floppy drive (see docs/qdev-device-use.txt:Default Devices), but actually doesn't. Technically that doc is just refering to the disablement of the primary floppy drive, as opposed to the floppy controller. The floppy controller itself is really a built-in device that is defined as part of the machine type, along with the various other platform devices hanging off the PIIX and ISA brige. I think you are right, this patch is a bit too harsh in fixing the problem: I just wanted to properly disable drive emulation, because from my tests the guest thinks that one drive is present even when is not. We should just be a little careful in explaining the difference between the controller, the drives, and what circumstances things show up and to whom. Currently the FDC is always present and always has two drive objects that are directly inlined to that device. Now, based on whether or not this line fires in vl.c: default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS); controls whether or not we find that drive in pc_basic_device_init as you've found, which controls (ultimately) whether or not fdctrl-drives[0].blk or fdctrl-drives[1].blk gets set. If the blk pointer is set, even if you have no media inserted, fd_revalidate (and pick_geometry) will *FORCIBLY PICK* a drive type for this floppy, currently a 1.44MB type, (20 sect, 80 track, 1 head) with a data rate of 500 Kbps. This is written to the rtc memory where seabios later reads it to discover if you have a guest-visible floppy drive there. Both QEMU and SeaBIOS confuse the concept of A drive is present with A disk is present which leads to these kinds of confusing scenarios. There's some work to do here, for sure. That was my impression too. On Thu, 14 May 2015, Stefan Weil wrote: Am 13.05.2015 um 20:15 schrieb Stefano Stabellini: On Wed, 13 May 2015, Daniel P. Berrange wrote: On Wed, May 13, 2015 at 06:29:46PM +0100, Stefano Stabellini wrote: Do not emulate a floppy drive if no drives are supposed to be present. This fixes the behavior of -nodefaults, that should remove the floppy drive (see docs/qdev-device-use.txt:Default Devices), but actually doesn't. Technically that doc is just refering to the disablement of the primary floppy drive, as opposed to the floppy controller. The floppy controller itself is really a built-in device that is defined as part of the machine type, along with the various other platform devices hanging off the PIIX and ISA brige. I think you are right, this patch is a bit too harsh in fixing the problem: I just wanted to properly disable drive emulation, because from my tests the guest thinks that one drive is present even when is not. A short test on some of my physical machines shows that most of them don't have a floppy disk controller at all (dmesg | grep FDC). Only some older machines still have one. Therefore I think that QEMU must also be able to offer a virtual machine without an FDC, maybe as the default for the next version of QEMU. I think it would make sense to make it the default for -nodefaults machines. I would be OK with a new property too, as we could set it from libxl or libvirt. Anybody would be happy to pick this one up or should I do it? ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On Thu, May 14, 2015 at 12:18:26PM +0100, Daniel P. Berrange wrote: On Thu, May 14, 2015 at 12:12:52PM +0100, Stefano Stabellini wrote: On Wed, 13 May 2015, John Snow wrote: On 05/13/2015 02:15 PM, Stefano Stabellini wrote: On Wed, 13 May 2015, Daniel P. Berrange wrote: On Wed, May 13, 2015 at 06:29:46PM +0100, Stefano Stabellini wrote: Do not emulate a floppy drive if no drives are supposed to be present. This fixes the behavior of -nodefaults, that should remove the floppy drive (see docs/qdev-device-use.txt:Default Devices), but actually doesn't. Technically that doc is just refering to the disablement of the primary floppy drive, as opposed to the floppy controller. The floppy controller itself is really a built-in device that is defined as part of the machine type, along with the various other platform devices hanging off the PIIX and ISA brige. I think you are right, this patch is a bit too harsh in fixing the problem: I just wanted to properly disable drive emulation, because from my tests the guest thinks that one drive is present even when is not. We should just be a little careful in explaining the difference between the controller, the drives, and what circumstances things show up and to whom. Currently the FDC is always present and always has two drive objects that are directly inlined to that device. Now, based on whether or not this line fires in vl.c: default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS); controls whether or not we find that drive in pc_basic_device_init as you've found, which controls (ultimately) whether or not fdctrl-drives[0].blk or fdctrl-drives[1].blk gets set. If the blk pointer is set, even if you have no media inserted, fd_revalidate (and pick_geometry) will *FORCIBLY PICK* a drive type for this floppy, currently a 1.44MB type, (20 sect, 80 track, 1 head) with a data rate of 500 Kbps. This is written to the rtc memory where seabios later reads it to discover if you have a guest-visible floppy drive there. Both QEMU and SeaBIOS confuse the concept of A drive is present with A disk is present which leads to these kinds of confusing scenarios. There's some work to do here, for sure. That was my impression too. On Thu, 14 May 2015, Stefan Weil wrote: Am 13.05.2015 um 20:15 schrieb Stefano Stabellini: On Wed, 13 May 2015, Daniel P. Berrange wrote: On Wed, May 13, 2015 at 06:29:46PM +0100, Stefano Stabellini wrote: Do not emulate a floppy drive if no drives are supposed to be present. This fixes the behavior of -nodefaults, that should remove the floppy drive (see docs/qdev-device-use.txt:Default Devices), but actually doesn't. Technically that doc is just refering to the disablement of the primary floppy drive, as opposed to the floppy controller. The floppy controller itself is really a built-in device that is defined as part of the machine type, along with the various other platform devices hanging off the PIIX and ISA brige. I think you are right, this patch is a bit too harsh in fixing the problem: I just wanted to properly disable drive emulation, because from my tests the guest thinks that one drive is present even when is not. A short test on some of my physical machines shows that most of them don't have a floppy disk controller at all (dmesg | grep FDC). Only some older machines still have one. Therefore I think that QEMU must also be able to offer a virtual machine without an FDC, maybe as the default for the next version of QEMU. I think it would make sense to make it the default for -nodefaults machines. I would be OK with a new property too, as we could set it from libxl or libvirt. Anybody would be happy to pick this one up or should I do it? It isn't permissible to change the hardware exposed to guests for existing machine types, even when -nodefaults is used. Any change in defaults has to be for new machine types only. eg we can't change pc-2.3.0 machine type, but we could change defaults for the pc-2.4.0 machine type that will be in next release. That ensures migration upgrade compatibility for existing guests, while letting new guests get the new defaults. Regards, Daniel For libvirt, yes. But command-line users might rely on the old behaviour with e.g. -M pc so we shouldn't change defaults lightly even for new machine types. -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ___ Xen-devel mailing list
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On Thu, May 14, 2015 at 12:12:52PM +0100, Stefano Stabellini wrote: I would be OK with a new property too, as we could set it from libxl or libvirt. Anybody would be happy to pick this one up or should I do it? Pls go ahead, I can merge it in the pc tree. -- MST ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On 05/13/2015 02:15 PM, Stefano Stabellini wrote: On Wed, 13 May 2015, Daniel P. Berrange wrote: On Wed, May 13, 2015 at 06:29:46PM +0100, Stefano Stabellini wrote: Do not emulate a floppy drive if no drives are supposed to be present. This fixes the behavior of -nodefaults, that should remove the floppy drive (see docs/qdev-device-use.txt:Default Devices), but actually doesn't. Technically that doc is just refering to the disablement of the primary floppy drive, as opposed to the floppy controller. The floppy controller itself is really a built-in device that is defined as part of the machine type, along with the various other platform devices hanging off the PIIX and ISA brige. I think you are right, this patch is a bit too harsh in fixing the problem: I just wanted to properly disable drive emulation, because from my tests the guest thinks that one drive is present even when is not. We should just be a little careful in explaining the difference between the controller, the drives, and what circumstances things show up and to whom. Currently the FDC is always present and always has two drive objects that are directly inlined to that device. Now, based on whether or not this line fires in vl.c: default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS); controls whether or not we find that drive in pc_basic_device_init as you've found, which controls (ultimately) whether or not fdctrl-drives[0].blk or fdctrl-drives[1].blk gets set. If the blk pointer is set, even if you have no media inserted, fd_revalidate (and pick_geometry) will *FORCIBLY PICK* a drive type for this floppy, currently a 1.44MB type, (20 sect, 80 track, 1 head) with a data rate of 500 Kbps. This is written to the rtc memory where seabios later reads it to discover if you have a guest-visible floppy drive there. Both QEMU and SeaBIOS confuse the concept of A drive is present with A disk is present which leads to these kinds of confusing scenarios. There's some work to do here, for sure. Anyway, maybe this patch is inappropriate: Just because we have no drives doesn't mean we don't have a controller. I have seen some discussion recently about allowing users to disable/remove the FDC altogether, though, and I think this is probably the way to go. diff --git a/hw/i386/pc.c b/hw/i386/pc.c index a8e6be1..c9f50b3 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1410,6 +1410,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, qemu_irq *cpu_exit_irq; MemoryRegion *ioport80_io = g_new(MemoryRegion, 1); MemoryRegion *ioportF0_io = g_new(MemoryRegion, 1); +bool floppy_exist; memory_region_init_io(ioport80_io, NULL, ioport80_io_ops, NULL, ioport80, 1); memory_region_add_subregion(isa_bus-address_space_io, 0x80, ioport80_io); @@ -1487,10 +1488,17 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, cpu_exit_irq = qemu_allocate_irqs(cpu_request_exit, NULL, 1); DMA_init(0, cpu_exit_irq); +*floppy = NULL; +floppy_exist = false; for(i = 0; i MAX_FD; i++) { fd[i] = drive_get(IF_FLOPPY, 0, i); +if (fd[i] != NULL) { +floppy_exist = true; +} +} +if (floppy_exist) { +*floppy = fdctrl_init_isa(isa_bus, fd); } -*floppy = fdctrl_init_isa(isa_bus, fd); } This results in a guest ABI change when updating QEMU, because the floppy controller will disappear for existing guests. This is liable to break guests upon migration. If we want to be able to disable the floppy controller itself, in addition to the floppy drives, then we'd likely need to define a property against the machine type to allow its existence to be toggled on/off. We'd then need to at least make sure the existing machine types defaulted to on, if we decided that new machine types should default to off. Libvirt would also need to gain the ability to represent the existance of the floppy controller and allow it to be turned on / off explicitly. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On Thu, May 14, 2015 at 06:38:24AM +0200, Stefan Weil wrote: Am 13.05.2015 um 20:15 schrieb Stefano Stabellini: On Wed, 13 May 2015, Daniel P. Berrange wrote: On Wed, May 13, 2015 at 06:29:46PM +0100, Stefano Stabellini wrote: Do not emulate a floppy drive if no drives are supposed to be present. This fixes the behavior of -nodefaults, that should remove the floppy drive (see docs/qdev-device-use.txt:Default Devices), but actually doesn't. Technically that doc is just refering to the disablement of the primary floppy drive, as opposed to the floppy controller. The floppy controller itself is really a built-in device that is defined as part of the machine type, along with the various other platform devices hanging off the PIIX and ISA brige. I think you are right, this patch is a bit too harsh in fixing the problem: I just wanted to properly disable drive emulation, because from my tests the guest thinks that one drive is present even when is not. A short test on some of my physical machines shows that most of them don't have a floppy disk controller at all (dmesg | grep FDC). Only some older machines still have one. Therefore I think that QEMU must also be able to offer a virtual machine without an FDC, maybe as the default for the next version of QEMU. Stefan Making it the default in QEMU might break a bunch of existing setups, but IMO it's very reasonable to add such a property, and teach libvirt to set it automatically. -- MST ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On Wed, May 13, 2015 at 06:29:46PM +0100, Stefano Stabellini wrote: Do not emulate a floppy drive if no drives are supposed to be present. This fixes the behavior of -nodefaults, that should remove the floppy drive (see docs/qdev-device-use.txt:Default Devices), but actually doesn't. Technically that doc is just refering to the disablement of the primary floppy drive, as opposed to the floppy controller. The floppy controller itself is really a built-in device that is defined as part of the machine type, along with the various other platform devices hanging off the PIIX and ISA brige. diff --git a/hw/i386/pc.c b/hw/i386/pc.c index a8e6be1..c9f50b3 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1410,6 +1410,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, qemu_irq *cpu_exit_irq; MemoryRegion *ioport80_io = g_new(MemoryRegion, 1); MemoryRegion *ioportF0_io = g_new(MemoryRegion, 1); +bool floppy_exist; memory_region_init_io(ioport80_io, NULL, ioport80_io_ops, NULL, ioport80, 1); memory_region_add_subregion(isa_bus-address_space_io, 0x80, ioport80_io); @@ -1487,10 +1488,17 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, cpu_exit_irq = qemu_allocate_irqs(cpu_request_exit, NULL, 1); DMA_init(0, cpu_exit_irq); +*floppy = NULL; +floppy_exist = false; for(i = 0; i MAX_FD; i++) { fd[i] = drive_get(IF_FLOPPY, 0, i); +if (fd[i] != NULL) { +floppy_exist = true; +} +} +if (floppy_exist) { +*floppy = fdctrl_init_isa(isa_bus, fd); } -*floppy = fdctrl_init_isa(isa_bus, fd); } This results in a guest ABI change when updating QEMU, because the floppy controller will disappear for existing guests. This is liable to break guests upon migration. If we want to be able to disable the floppy controller itself, in addition to the floppy drives, then we'd likely need to define a property against the machine type to allow its existence to be toggled on/off. We'd then need to at least make sure the existing machine types defaulted to on, if we decided that new machine types should default to off. Libvirt would also need to gain the ability to represent the existance of the floppy controller and allow it to be turned on / off explicitly. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Do not emulate a floppy drive when -nodefaults
On Wed, 13 May 2015, Daniel P. Berrange wrote: On Wed, May 13, 2015 at 06:29:46PM +0100, Stefano Stabellini wrote: Do not emulate a floppy drive if no drives are supposed to be present. This fixes the behavior of -nodefaults, that should remove the floppy drive (see docs/qdev-device-use.txt:Default Devices), but actually doesn't. Technically that doc is just refering to the disablement of the primary floppy drive, as opposed to the floppy controller. The floppy controller itself is really a built-in device that is defined as part of the machine type, along with the various other platform devices hanging off the PIIX and ISA brige. I think you are right, this patch is a bit too harsh in fixing the problem: I just wanted to properly disable drive emulation, because from my tests the guest thinks that one drive is present even when is not. diff --git a/hw/i386/pc.c b/hw/i386/pc.c index a8e6be1..c9f50b3 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1410,6 +1410,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, qemu_irq *cpu_exit_irq; MemoryRegion *ioport80_io = g_new(MemoryRegion, 1); MemoryRegion *ioportF0_io = g_new(MemoryRegion, 1); +bool floppy_exist; memory_region_init_io(ioport80_io, NULL, ioport80_io_ops, NULL, ioport80, 1); memory_region_add_subregion(isa_bus-address_space_io, 0x80, ioport80_io); @@ -1487,10 +1488,17 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, cpu_exit_irq = qemu_allocate_irqs(cpu_request_exit, NULL, 1); DMA_init(0, cpu_exit_irq); +*floppy = NULL; +floppy_exist = false; for(i = 0; i MAX_FD; i++) { fd[i] = drive_get(IF_FLOPPY, 0, i); +if (fd[i] != NULL) { +floppy_exist = true; +} +} +if (floppy_exist) { +*floppy = fdctrl_init_isa(isa_bus, fd); } -*floppy = fdctrl_init_isa(isa_bus, fd); } This results in a guest ABI change when updating QEMU, because the floppy controller will disappear for existing guests. This is liable to break guests upon migration. If we want to be able to disable the floppy controller itself, in addition to the floppy drives, then we'd likely need to define a property against the machine type to allow its existence to be toggled on/off. We'd then need to at least make sure the existing machine types defaulted to on, if we decided that new machine types should default to off. Libvirt would also need to gain the ability to represent the existance of the floppy controller and allow it to be turned on / off explicitly. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel