Daniel P. Berrangé wrote: > On Wed, Feb 06, 2019 at 04:00:06PM -0500, Cole Robinson wrote: > > On 1/25/19 12:54 PM, Roman Bogorodskiy wrote: > > > > > > Roman Bogorodskiy (3): > > > conf: introduce 'msrs' feature > > > bhyve: implement MSRs ignore unknown writes feature > > > news: document bhyve msrs feature > > > > > > docs/drvbhyve.html.in | 16 +++++++++ > > > docs/formatdomain.html.in | 1 + > > > docs/news.xml | 11 ++++++ > > > docs/schemas/domaincommon.rng | 14 ++++++++ > > > src/bhyve/bhyve_command.c | 4 +++ > > > src/conf/domain_conf.c | 33 +++++++++++++++++ > > > src/conf/domain_conf.h | 8 +++++ > > > src/qemu/qemu_domain.c | 1 + > > > .../bhyvexml2argvdata/bhyvexml2argv-msrs.args | 10 ++++++ > > > .../bhyvexml2argv-msrs.ldargs | 3 ++ > > > .../bhyvexml2argvdata/bhyvexml2argv-msrs.xml | 26 ++++++++++++++ > > > tests/bhyvexml2argvtest.c | 1 + > > > .../bhyvexml2xmlout-msrs.xml | 36 +++++++++++++++++++ > > > tests/bhyvexml2xmltest.c | 1 + > > > 14 files changed, 165 insertions(+) > > > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-msrs.args > > > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-msrs.ldargs > > > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-msrs.xml > > > create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-msrs.xml > > > > > > > The code looks fine to me but this needs a bit more context. > > Particularly 'ignore' is a bit ambiguous here so I had to dig > > > > kvm+linux has arguably always ignored unknown MSRs but historically it > > would print an error in host dmesg which often confused users quite a > > bit. In the bhyve case it seems that unknown MSRs cause the VM to > > essentially crash which is a pretty intense reaction. This option > > disables that crashing behavior. So for this feature to be really > > descriptive it would be <msrs crashOnUnknown='yes|no' /> or something > > like that > > That is not actually the case for KVM. When KVM sees an unhandled > MSR it will inject a GPF to the guest, which will usually cause the > guest to crash. KVM does this GPF injection for both reads and writes > of unknown MSRs. > > There is an opt-in kvm.ko option "ignore-msrs" which tells KVM > to silently ignore unknown MSRs instead. Ideally the KVM option > would be exposed to QEMU as an option, since when people do need > it, they usually only need it for a single guest. > > > The bhyve man page says: > > > > https://www.freebsd.org/cgi/man.cgi?query=bhyve&sektion=8 > > > > -w Ignore accesses to unimplemented Model Specific Registers > > (MSRs). This is intended for debug purposes. > > I'm curious whether this applies to boths reads & writes of unknown > MSRs, or just to writes. '-w' naming suggests the latter, but the > descrption "accesses" would suggest reads too. From what I can see by briefly looking at the code, it applies to both reads and writes: https://svnweb.freebsd.org/base/head/usr.sbin/bhyve/bhyverun.c?revision=343642&view=markup#l520 Check two functions below this line. > > > > Calling it 'intended for debug purposes' also makes me question whether > > this should be encoded in the libvirt XML or just worked around with > > commandline passthrough > > > > (IMO To be friendly to users this option should be enabled by default > > but obviously that's against the intention of bhyve devs...) > > Enabling it by default is not a good idea as it can lead to silently > incorrect guest OS behaviour. That could lead to all manner of bad > things depending on the MSR in question. > > > ccing danpb to see if he has any thoughs about exposing this in the XML > > My only thought is the naming & whether we have one attribute for > ignoring reads & writes, or separate attributes. > > Regards, > Daniel > -- > |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| Roman Bogorodskiy
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
