[PATCH] qemu-xen: Fix PV segfault

[PATCH] qemu-xen: Fix PV segfault

[Kevin Wolf]

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

Don't access cur_cpu with CONFIG_DM, for PV machines it is NULL.

Signed-off-by: Kevin Wolf <kwolf@suse.de>

[-- Attachment #2: fix-pv-segfault.patch --]
[-- Type: all/allfiles, Size: 1086 bytes --]

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] qemu-xen: Fix PV segfault

[Kevin Wolf]

Hi Ian,

anything wrong with this one? I neither got an answer nor did you commit
to the repository. Or is it just me who still got to get used to git?

Kevin


Kevin Wolf schrieb:
> Don't access cur_cpu with CONFIG_DM, for PV machines it is NULL.
> 
> Signed-off-by: Kevin Wolf <kwolf@suse.de>
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] qemu-xen: Fix PV segfault

[Ian Jackson]

Kevin Wolf writes ("[PATCH] qemu-xen: Fix PV segfault"):
> Don't access cur_cpu with CONFIG_DM, for PV machines it is NULL.

I'm not sure I understand the execution context here properly.  Is
this for blktap ?  It would be better to make cur_cpu be set to some
sensible value.

Ian.

Re: [PATCH] qemu-xen: Fix PV segfault

[Kevin Wolf]

Ian Jackson schrieb:
> Kevin Wolf writes ("[PATCH] qemu-xen: Fix PV segfault"):
>> Don't access cur_cpu with CONFIG_DM, for PV machines it is NULL.
> 
> I'm not sure I understand the execution context here properly.  Is
> this for blktap ?  It would be better to make cur_cpu be set to some
> sensible value.

No, this is completely independent of blktap. Currently, PV machines
just don't have a virtual cpu in qemu - in contrary to FV machines which
call a complete pc_machine.init() during their initialization.

Maybe you're right and it would be better to assign even PV machines a
cpu, but I'm not sure what other functionality this implies to
implement. I'll try to look into this.

Kevin

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Gerd Hoffmann]

Kevin Wolf wrote:
> Maybe you're right and it would be better to assign even PV machines a
> cpu, but I'm not sure what other functionality this implies to
> implement. I'll try to look into this.

You can create a dummy cpu in halted state, have a look at
http://kraxel.fedorapeople.org/patches/0004-xen-groundwork-for-xen-support.patch
;)

While I'm at it:  There is a patch queue with at
http://kraxel.fedorapeople.org/patches/

The plan is to submit that stuff upstream, so regular qemu will have xen
support.  Right now the patch queue has only paravirt bits.  It comes
with a new machine type and a bunch of backend drivers (console,
framebuffer, disk, nic).  Will be posted to the list when it is ready
for submission (framebuffer still has some problems).

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[John Levon]

On Wed, Jul 02, 2008 at 10:38:32AM +0200, Gerd Hoffmann wrote:

> While I'm at it:  There is a patch queue with at
> http://kraxel.fedorapeople.org/patches/
> 
> The plan is to submit that stuff upstream, so regular qemu will have xen
> support.  Right now the patch queue has only paravirt bits.  It comes

I'm confused, I thought that was the point of the qemu-xen tree

regards
john

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Ian Jackson]

John Levon writes ("Re: [Xen-devel] Re: [PATCH] qemu-xen: Fix PV segfault"):
> On Wed, Jul 02, 2008 at 10:38:32AM +0200, Gerd Hoffmann wrote:
> > While I'm at it:  There is a patch queue with at
> > http://kraxel.fedorapeople.org/patches/
> > 
> > The plan is to submit that stuff upstream, so regular qemu will have xen
> > support.  Right now the patch queue has only paravirt bits.  It comes
> 
> I'm confused, I thought that was the point of the qemu-xen tree

Gerd Hoffman's tree seems to be something entirely different.  I'm not
sure I understand what the point of it is.  The reason why everyone is
using qemu is that it has all of the hardware emulation device models.

Qemu proper (ie upstream) also uses the CPU emulator (translator,
now).  KVM and Xen don't use that because they run the code on the
physical CPU one way or another.

Gerd Hoffman's setup doesn't use the device models, nor the CPU
emulator.  Really as far as I can tell the only thing he wants from
qemu is the command line parser !  This is quite bizarre as it's not a
very good command line parser (qemu upstream are considering replacing
it with something more data-driven and more modular).

Ian.

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Stefan de Konink]

Ian Jackson schreef:
> John Levon writes ("Re: [Xen-devel] Re: [PATCH] qemu-xen: Fix PV segfault"):
>> On Wed, Jul 02, 2008 at 10:38:32AM +0200, Gerd Hoffmann wrote:
>>> While I'm at it:  There is a patch queue with at
>>> http://kraxel.fedorapeople.org/patches/
>>>
>>> The plan is to submit that stuff upstream, so regular qemu will have xen
>>> support.  Right now the patch queue has only paravirt bits.  It comes
>> I'm confused, I thought that was the point of the qemu-xen tree
> 
> Gerd Hoffman's tree seems to be something entirely different.  I'm not
> sure I understand what the point of it is.  The reason why everyone is
> using qemu is that it has all of the hardware emulation device models.
> 
> Qemu proper (ie upstream) also uses the CPU emulator (translator,
> now).  KVM and Xen don't use that because they run the code on the
> physical CPU one way or another.
> 
> Gerd Hoffman's setup doesn't use the device models, nor the CPU
> emulator.  Really as far as I can tell the only thing he wants from
> qemu is the command line parser !  This is quite bizarre as it's not a
> very good command line parser (qemu upstream are considering replacing
> it with something more data-driven and more modular).

I think the primary idea is to get away from xend, and qemu gets the job 
done.


Stefan

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[John Levon]

On Wed, Jul 02, 2008 at 03:32:15PM +0100, Ian Jackson wrote:

> > > While I'm at it:  There is a patch queue with at
> > > http://kraxel.fedorapeople.org/patches/
> > > 
> > > The plan is to submit that stuff upstream, so regular qemu will have xen
> > > support.  Right now the patch queue has only paravirt bits.  It comes
> > 
> > I'm confused, I thought that was the point of the qemu-xen tree
> 
> Gerd Hoffman's tree seems to be something entirely different.  I'm not
> sure I understand what the point of it is.  The reason why everyone is
> using qemu is that it has all of the hardware emulation device models.

Oh, if it's just the PV bits, then it's probably part of the work to
drop the need for the Xen daemons altogether and move to a domain ==
running qemu implementation as Dan Berrange presented at the summit some
time ago.

This approach has some big advantages (a lot less code) but yeah, it is
a little weird.

regards
john

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Ian Jackson]

John Levon writes ("Re: [Xen-devel] Re: [PATCH] qemu-xen: Fix PV segfault"):
> Oh, if it's just the PV bits, then it's probably part of the work to
> drop the need for the Xen daemons altogether and move to a domain ==
> running qemu implementation as Dan Berrange presented at the summit some
> time ago.

I wasn't around for that summit presentation but for the record my
personal view is that this is a bad idea.  dom0 host processes are
much more fragile (much more vulnerable to failures induced both
inside that process and from the rest of dom0) than a Xen domain.
Ideally the proper functioning of guests would not depend on that kind
of complexity.

Indeed currently even if dom0 entirely stops running user code for
some reason, it is still possible to have PV guests keep running and
cleanly shut themselves down (although management functions like
migration, device hotplug, and requesting shutdown from dom0 are of
course unavailable).

So guests should continue to be regarded as owned and parented by the
hypervisor, not by some dom0 userland process.

Ian.

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Kevin Wolf]

Ian Jackson schrieb:
> John Levon writes ("Re: [Xen-devel] Re: [PATCH] qemu-xen: Fix PV segfault"):
>> Oh, if it's just the PV bits, then it's probably part of the work to
>> drop the need for the Xen daemons altogether and move to a domain ==
>> running qemu implementation as Dan Berrange presented at the summit some
>> time ago.
> 
> I wasn't around for that summit presentation but for the record my
> personal view is that this is a bad idea.  dom0 host processes are
> much more fragile (much more vulnerable to failures induced both
> inside that process and from the rest of dom0) than a Xen domain.
> Ideally the proper functioning of guests would not depend on that kind
> of complexity.
> 
> Indeed currently even if dom0 entirely stops running user code for
> some reason, it is still possible to have PV guests keep running and
> cleanly shut themselves down (although management functions like
> migration, device hotplug, and requesting shutdown from dom0 are of
> course unavailable).
> 
> So guests should continue to be regarded as owned and parented by the
> hypervisor, not by some dom0 userland process.

They would still be owned by the hypervisor (anything else wouldn't be
Xen any more). It's just the Dom0 tools which could be accumulated in
qemu. That is, you wouldn't use xm create and xend and all that stuff,
but start qemu to get the domain set up and running.

I'm not completely sure if I really got the idea, but that's what I
understand.

Kevin

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Stefan de Konink]

Ian Jackson schreef:
> So guests should continue to be regarded as owned and parented by the
> hypervisor, not by some dom0 userland process.

But this isn't the case now at all, is it? I name xenstored; is the move 
that Samuel with minios takes a way to solve this?

In any case, if anyone suggests that userspace processes are bad and 
fragile, then these persons will not like any processes that support 
hardware emulation, such as tapdisk.


Stefan

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Ian Jackson]

Stefan de Konink writes ("Re: [Xen-devel] Re: [PATCH] qemu-xen: Fix PV segfault"):
> But this isn't the case now at all, is it? I name xenstored; is the move 
> that Samuel with minios takes a way to solve this?

xenstored is not critical to the run-time operation of PV domains.
Without it you can't boot, add devices, etc. etc.  But you can (for
example) shut down a PV domain cleanly.

> In any case, if anyone suggests that userspace processes are bad and 
> fragile, then these persons will not like any processes that support 
> hardware emulation, such as tapdisk.

You're quite right.

Ian.

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Stefan de Konink]

Ian Jackson schreef:
> Stefan de Konink writes ("Re: [Xen-devel] Re: [PATCH] qemu-xen: Fix PV segfault"):
>> But this isn't the case now at all, is it? I name xenstored; is the move 
>> that Samuel with minios takes a way to solve this?
> 
> xenstored is not critical to the run-time operation of PV domains.
> Without it you can't boot, add devices, etc. etc.  But you can (for
> example) shut down a PV domain cleanly.

So the actual 'problem' is the storage of the domain properties, 
preferably in a 'process' that is not running, but an API that can query 
it. I think Libvirtd fails on that point too, everything gets fubar, 
once a daemon breaks. The reason for a running daemon providing domain 
specific information is not clear for me, storing it on disk will always 
give better recovery possibilities.

>> In any case, if anyone suggests that userspace processes are bad and 
>> fragile, then these persons will not like any processes that support 
>> hardware emulation, such as tapdisk.
> 
> You're quite right.

If tapdisk is a problem, then I'm happy to point at the breakage that 
will occur with loopback devices (NFS + loopback + highio = boom). But 
it is really interesting people talk about 'user-processes' as if they 
are broken by default. I disagree, even if a iscsi setup for each used 
disk a kernel process is running. So one way or the other, dom0 is 
needed to provide access to storage one way or the other. Migrating the 
'storage providers' to DomS would be an interesting idea, but the need 
for it, I don't quite see.


Stefan

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Daniel P. Berrange]

On Wed, Jul 02, 2008 at 05:28:45PM +0200, Stefan de Konink wrote:
> Ian Jackson schreef:
> >Stefan de Konink writes ("Re: [Xen-devel] Re: [PATCH] qemu-xen: Fix PV 
> >segfault"):
> >>But this isn't the case now at all, is it? I name xenstored; is the move 
> >>that Samuel with minios takes a way to solve this?
> >
> >xenstored is not critical to the run-time operation of PV domains.
> >Without it you can't boot, add devices, etc. etc.  But you can (for
> >example) shut down a PV domain cleanly.
> 
> So the actual 'problem' is the storage of the domain properties, 
> preferably in a 'process' that is not running, but an API that can query 
> it. I think Libvirtd fails on that point too, everything gets fubar, 
> once a daemon breaks. The reason for a running daemon providing domain 
> specific information is not clear for me, storing it on disk will always 
> give better recovery possibilities.

That is just an artifact of libvirt being written to work with all QEMU
versions back to 0.8.1, which didn't have a good way to re-connect to
the monitor of a running VM. Its on the TODO list to automatically use
a UNIX socket for monitor communication when we detect QEMU >= 0.9.x
so there's no need for libvirt  be the direct parent of the QEMU processes

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[John Levon]

On Wed, Jul 02, 2008 at 04:14:47PM +0100, Ian Jackson wrote:

> Stefan de Konink writes ("Re: [Xen-devel] Re: [PATCH] qemu-xen: Fix PV segfault"):
> > But this isn't the case now at all, is it? I name xenstored; is the move 
> > that Samuel with minios takes a way to solve this?
> 
> xenstored is not critical to the run-time operation of PV domains.

It's critical enough: no suspend/resume, no device hotplug, etc.

> Without it you can't boot, add devices, etc. etc.  But you can (for
> example) shut down a PV domain cleanly.

Really, how? There's no-one to write the control/shutdown node etc.

regards
john

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Ian Jackson]

John Levon writes ("Re: [Xen-devel] Re: [PATCH] qemu-xen: Fix PV segfault"):
> Really, how? There's no-one to write the control/shutdown node etc.

Log into the guest and tell it to shut itself down, obviously !

Ian.

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Daniel P. Berrange]

On Wed, Jul 02, 2008 at 03:50:15PM +0100, Ian Jackson wrote:
> John Levon writes ("Re: [Xen-devel] Re: [PATCH] qemu-xen: Fix PV segfault"):
> > Oh, if it's just the PV bits, then it's probably part of the work to
> > drop the need for the Xen daemons altogether and move to a domain ==
> > running qemu implementation as Dan Berrange presented at the summit some
> > time ago.
> 
> I wasn't around for that summit presentation but for the record my
> personal view is that this is a bad idea.  dom0 host processes are
> much more fragile (much more vulnerable to failures induced both
> inside that process and from the rest of dom0) than a Xen domain.
> Ideally the proper functioning of guests would not depend on that kind
> of complexity.
> 
> Indeed currently even if dom0 entirely stops running user code for
> some reason, it is still possible to have PV guests keep running and
> cleanly shut themselves down (although management functions like
> migration, device hotplug, and requesting shutdown from dom0 are of
> course unavailable).
> 
> So guests should continue to be regarded as owned and parented by the
> hypervisor, not by some dom0 userland process.

My work didn't change who owned the VM's at all. I literally just took
the the domain setup code from the constructDomain() method in XenDomainInfo.py
and replicated it in QEMU. So instead of having XenD construct the domain,
QEMU did it directly. The split of responsibilities between HV vs Dom0
remained the same - it was just shuffling responsibilities within Dom0.

The original patches (no longer working against current xen-unstable)
are here

   http://hg.berrange.com/tools/xenite--devel

The main problem I didn't solve was correct cleanup in the event that
QEMU crashed - something XenD would ordinarily do - but would have to
be done by whatever was launching QEMU, or perhaps via a Dom0 kernel
hook.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Daniel P. Berrange]

On Wed, Jul 02, 2008 at 03:50:15PM +0100, Ian Jackson wrote:
> John Levon writes ("Re: [Xen-devel] Re: [PATCH] qemu-xen: Fix PV segfault"):
> > Oh, if it's just the PV bits, then it's probably part of the work to
> > drop the need for the Xen daemons altogether and move to a domain ==
> > running qemu implementation as Dan Berrange presented at the summit some
> > time ago.
> 
> I wasn't around for that summit presentation but for the record my
> personal view is that this is a bad idea.  dom0 host processes are
> much more fragile (much more vulnerable to failures induced both
> inside that process and from the rest of dom0) than a Xen domain.
> Ideally the proper functioning of guests would not depend on that kind
> of complexity.

FYI I also wrote a short report outlining what I was trying to achieve.
I was in fact trying to reduce the complexity of userspace processes
in Xen, and to try to take maximum advantage of QEMU's existing code
for stuff like VNC, text consoles, device emulation, bootloading, and
all the disk formats

http://people.redhat.com/berrange/xen-summit-2007-sj/xen-summit-xenite-report.pdf

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[John Levon]

On Wed, Jul 02, 2008 at 03:50:15PM +0100, Ian Jackson wrote:

> I wasn't around for that summit presentation but for the record my
> personal view is that this is a bad idea.  dom0 host processes are
> much more fragile (much more vulnerable to failures induced both
> inside that process and from the rest of dom0) than a Xen domain.
> Ideally the proper functioning of guests would not depend on that kind
> of complexity.

Indeed. However, this is a fact now for HVM. So making it be true for PV
too isn't really the worst thing.

What needs to happen is to work out some way of making qemu restartable.
I have no idea how feasible that really is. (The same applies to all of
dom0, user and kernel space.)

regards
john

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Kevin Wolf]

Ian Jackson schrieb:
> Gerd Hoffman's setup doesn't use the device models, nor the CPU
> emulator.  Really as far as I can tell the only thing he wants from
> qemu is the command line parser !  This is quite bizarre as it's not a
> very good command line parser (qemu upstream are considering replacing
> it with something more data-driven and more modular).

Actually, we're not using the device model for PV either. Apart from the
command line parser we're getting at least the graphics and (with
tap:ioemu) the whole image handling stuff from qemu. For HVM, obviously,
we're using the device model, and I assume that Gerd will do that, too.

As John says, I think this approach (let qemu handle everything) could
indeed make sense because it has the potential to make things much simpler.

Kevin

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Daniel P. Berrange]

On Wed, Jul 02, 2008 at 03:32:15PM +0100, Ian Jackson wrote:
> John Levon writes ("Re: [Xen-devel] Re: [PATCH] qemu-xen: Fix PV segfault"):
> > On Wed, Jul 02, 2008 at 10:38:32AM +0200, Gerd Hoffmann wrote:
> > > While I'm at it:  There is a patch queue with at
> > > http://kraxel.fedorapeople.org/patches/
> > > 
> > > The plan is to submit that stuff upstream, so regular qemu will have xen
> > > support.  Right now the patch queue has only paravirt bits.  It comes
> > 
> > I'm confused, I thought that was the point of the qemu-xen tree
> 
> Gerd Hoffman's tree seems to be something entirely different.  I'm not
> sure I understand what the point of it is.  The reason why everyone is
> using qemu is that it has all of the hardware emulation device models.
> 
> Qemu proper (ie upstream) also uses the CPU emulator (translator,
> now).  KVM and Xen don't use that because they run the code on the
> physical CPU one way or another.
> 
> Gerd Hoffman's setup doesn't use the device models, nor the CPU
> emulator.  Really as far as I can tell the only thing he wants from
> qemu is the command line parser !  This is quite bizarre as it's not a
> very good command line parser (qemu upstream are considering replacing
> it with something more data-driven and more modular).

It is a derivative of work I did previously to allow qemu-dm to be run
standalone from XenD for starting Xen guests. My proof of concept allowed
you to run both paravirt and fullyvirt guests just be invoking

   qemu-xen -M xenpv -hda /some/path -r 500 

(Change xenpv to xenfv for fullyvirt).  In PV mode it leveraged QEMU for
the graphical framebuffer and text console, to allow VNC and all the
text console options. In FV mode it leveraged all the emulated devices
too. 

Gerd's new code is followig the same idea as my original work but is against
upstream QEMU, instead of ioemu. At this time it only supports PV so is really
only using the CLI parser and text console / VNC server pieces. I believe
he is still planning to add in FV too.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Gerd Hoffmann]

Ian Jackson wrote:
> Gerd Hoffman's tree seems to be something entirely different.  I'm not
> sure I understand what the point of it is.  The reason why everyone is
> using qemu is that it has all of the hardware emulation device models.

And everybody is forking qemu for that which is bad.

If some security issue shows up distros have to update tons of packages,
each with a copy of qemu, to name only one problem with that.

> Qemu proper (ie upstream) also uses the CPU emulator (translator,
> now).  KVM and Xen don't use that because they run the code on the
> physical CPU one way or another.

Yep.

> Gerd Hoffman's setup doesn't use the device models, nor the CPU
> emulator.  Really as far as I can tell the only thing he wants from
> qemu is the command line parser !  This is quite bizarre as it's not a
> very good command line parser (qemu upstream are considering replacing
> it with something more data-driven and more modular).

If you start a pv guest with a paravirt framebuffer a qemu-dm process in
dom0 will handle the framebuffer backend and the vnc server.  I want
those bits being merged into upstream qemu, so upstream qemu is able to
do that too.

At very minimum I want those parts needed by xenner in upstream qemu,
which is just the pv bits.  Merging all the hvm stuff too and thereby
making the qemu-dm fork obsolete would be cool too.

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Gerd Hoffmann]

John Levon wrote:
> On Wed, Jul 02, 2008 at 10:38:32AM +0200, Gerd Hoffmann wrote:
> 
>> While I'm at it:  There is a patch queue with at
>> http://kraxel.fedorapeople.org/patches/
>>
>> The plan is to submit that stuff upstream, so regular qemu will have xen
>> support.  Right now the patch queue has only paravirt bits.  It comes
> 
> I'm confused, I thought that was the point of the qemu-xen tree

Looked at the qemu-xen tree and the branches therein today (missed the
original announcement as I was on vacation at that point in time).
Ian's plan seems to be to upstream mostly bugfixes (qemu branch), not
any xen-specific code.

I also want to have the xen bits upstream (pv first as mentioned in the
previous mail).  And I want to have it as runtime option, not
compile-time option, so you don't have a separate qemu-dm binary.

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Kevin Wolf]

Gerd Hoffmann schrieb:
> I also want to have the xen bits upstream (pv first as mentioned in the
> previous mail).  And I want to have it as runtime option, not
> compile-time option, so you don't have a separate qemu-dm binary.

Actually, I think Xen PV is a different architecture, not just a
different PC machine. So to be consistent with qemu it would be a
separate binary like qemu-system-xenpv.

HVM on the other hand would be more of an accelerator for the existing
x86 system emulation, just like kqemu or KVM.

Kevin

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Gerd Hoffmann]

Kevin Wolf wrote:
> Actually, I think Xen PV is a different architecture, not just a
> different PC machine.

That isn't visible to qemu though as cpu virtualization is handled by
the xen hypervisor.

> So to be consistent with qemu it would be a
> separate binary like qemu-system-xenpv.

I wouldn't do it that way for the reason outlined above, but it is an
option, yes.

> HVM on the other hand would be more of an accelerator for the existing
> x86 system emulation, just like kqemu or KVM.

Yep, sort of.  And some plumbing for xenbus for guests with pv-on-hvm
drivers installed.

Glauber Costa is working on a nice abstraction layer for qemu
accelerators btw.  Intention is to get kvm upstream merged.  It should
also make it easier to get xen hvm support merged upstream.

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Kevin Wolf]

Gerd Hoffmann schrieb:
> Kevin Wolf wrote:
>> Actually, I think Xen PV is a different architecture, not just a
>> different PC machine.
> 
> That isn't visible to qemu though as cpu virtualization is handled by
> the xen hypervisor.
> 
>> So to be consistent with qemu it would be a
>> separate binary like qemu-system-xenpv.
> 
> I wouldn't do it that way for the reason outlined above, but it is an
> option, yes.

It's not only the CPU virtualization I'm talking about. PV machines also
have "hardware" devices completely different from PCs. Just look at
pc_init1(), that's all stuff PV machines won't need.

Concerning the CPU, I think of it as an architecture which is only
usable through an accelerator. Certainly it would be possible to do a
normal qemu emulation for this architecture, it just wouldn't make too
much sense.

>> HVM on the other hand would be more of an accelerator for the existing
>> x86 system emulation, just like kqemu or KVM.
> 
> Yep, sort of.  And some plumbing for xenbus for guests with pv-on-hvm
> drivers installed.
> 
> Glauber Costa is working on a nice abstraction layer for qemu
> accelerators btw.  Intention is to get kvm upstream merged.  It should
> also make it easier to get xen hvm support merged upstream.

Yes, I'm aware of his patches. That's actually why I used the term
"accelerator".

Kevin

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Kevin Wolf]

Kevin Wolf schrieb:
> Gerd Hoffmann schrieb:
>> Kevin Wolf wrote:
>>> Actually, I think Xen PV is a different architecture, not just a
>>> different PC machine.
>> That isn't visible to qemu though as cpu virtualization is handled by
>> the xen hypervisor.
>>
>>> So to be consistent with qemu it would be a
>>> separate binary like qemu-system-xenpv.
>> I wouldn't do it that way for the reason outlined above, but it is an
>> option, yes.
> 
> It's not only the CPU virtualization I'm talking about. PV machines also
> have "hardware" devices completely different from PCs. Just look at
> pc_init1(), that's all stuff PV machines won't need.

Seems the temperature prevents clear thinking. Forget about the hardware
devices, that's exactly what the different machines are for...

Kevin

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Gerd Hoffmann]

Kevin Wolf wrote:
> It's not only the CPU virtualization I'm talking about. PV machines also
> have "hardware" devices completely different from PCs. Just look at
> pc_init1(), that's all stuff PV machines won't need.

Sure.  On the other hand the pv (aka xenbus) devices are not strictly
limited to pv domains, so these two worlds are not separated but have
some overlaps.

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Ian Jackson]

Gerd Hoffmann writes ("Re: [Xen-devel] Re: [PATCH] qemu-xen: Fix PV segfault"):
> Looked at the qemu-xen tree and the branches therein today (missed the
> original announcement as I was on vacation at that point in time).
> Ian's plan seems to be to upstream mostly bugfixes (qemu branch), not
> any xen-specific code.

I would like the xen code to go upstream as well eventually.  However
upstream have said that they're not happy with some of the things
which are essential in our tree.  For example, they are opposed to the
mapcache, which is essential for decoupling guest addresses from
qemu's address space (particularly since the guest maybe 64-bit and
qemu only 32-bit!)

> I also want to have the xen bits upstream (pv first as mentioned in the
> previous mail).  And I want to have it as runtime option, not
> compile-time option, so you don't have a separate qemu-dm binary.

If upstream change their mind then this would be nice.  But in the
absence of that, I think a different structure is called for: we
should attempt to make a separation between the parts of qemu we reuse
and the Xen stuff we add, and try to make the parts of qemu we need to
reuse but modify as small as possible.

Ian.

Re: Re: [PATCH] qemu-xen: Fix PV segfault

[Gerd Hoffmann]

Ian Jackson wrote:
> Gerd Hoffmann writes ("Re: [Xen-devel] Re: [PATCH] qemu-xen: Fix PV segfault"):
>> Looked at the qemu-xen tree and the branches therein today (missed the
>> original announcement as I was on vacation at that point in time).
>> Ian's plan seems to be to upstream mostly bugfixes (qemu branch), not
>> any xen-specific code.
> 
> I would like the xen code to go upstream as well eventually.  However
> upstream have said that they're not happy with some of the things
> which are essential in our tree.  For example, they are opposed to the
> mapcache, which is essential for decoupling guest addresses from
> qemu's address space (particularly since the guest maybe 64-bit and
> qemu only 32-bit!)

That doesn't mean we can't upstream any xen bits.  The whole pv domain
support doesn't need mapcache in the first place for example.

Didn't check how intrusive mapcache is, but would it be an option to
make it optional?  So upstream has most other bits, you have a much
smaller patch queue.

Also I'd expect that if you run guests that big that the 32bit address
space limits become problematic, wouldn't you run 64bit xen+dom0 anyway
for performance reasons?

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

Re: [PATCH] qemu-xen: Fix PV segfault

[Ian Jackson]

Kevin Wolf writes ("Re: [PATCH] qemu-xen: Fix PV segfault"):
> Ian Jackson schrieb:
> > Kevin Wolf writes ("[PATCH] qemu-xen: Fix PV segfault"):
> >> Don't access cur_cpu with CONFIG_DM, for PV machines it is NULL.
> > 
> > I'm not sure I understand the execution context here properly.  Is
> > this for blktap ?  It would be better to make cur_cpu be set to some
> > sensible value.
> 
> No, this is completely independent of blktap. Currently, PV machines
> just don't have a virtual cpu in qemu - in contrary to FV machines which
> call a complete pc_machine.init() during their initialization.

Evidently this is a configuration I haven't tested.  My PV machines
don't use a qemu-dm at all.

> Maybe you're right and it would be better to assign even PV machines a
> cpu, but I'm not sure what other functionality this implies to
> implement. I'll try to look into this.

This isn't very hard, surely ?  The problem is presumably that the PV
startup is bypassing some of the necessary initialisation.  I
encountered this myself for the HVM case.

It is quite fine for qemu to have what it thinks of as a real CPU but
which isn't really used.  That's how the HVM guests work anyway.  All
that's needed is enough of a cheap plastic imitation that these kind
of enquiry calls (flag checks, etc.) don't die and give plausible
answers.

I would very much prefer an answer which didn't make any additional
changes to vl.c (or other code shared with upstream).  If such changes
are necessary they should be textually small.  This will make keeping
us up to date with qemu upstream much easier.

Ian.

Re: [PATCH] qemu-xen: Fix PV segfault

[Kevin Wolf]

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

Ian Jackson schrieb:
> Kevin Wolf writes ("Re: [PATCH] qemu-xen: Fix PV segfault"):
>> Ian Jackson schrieb:
>>> Kevin Wolf writes ("[PATCH] qemu-xen: Fix PV segfault"):
>>>> Don't access cur_cpu with CONFIG_DM, for PV machines it is NULL.
>>> I'm not sure I understand the execution context here properly.  Is
>>> this for blktap ?  It would be better to make cur_cpu be set to some
>>> sensible value.
>> No, this is completely independent of blktap. Currently, PV machines
>> just don't have a virtual cpu in qemu - in contrary to FV machines which
>> call a complete pc_machine.init() during their initialization.
> 
> Evidently this is a configuration I haven't tested.  My PV machines
> don't use a qemu-dm at all.

AFAIK, you automatically get qemu-dm as soon as you use VNC. So this is
probably the most common use case.

While I'm at it: There's another problem with qemu-xen, I can establish
a VNC connection only if I was connected via xm console before. Maybe I
can take a look at this later this week.

> I would very much prefer an answer which didn't make any additional
> changes to vl.c (or other code shared with upstream).  If such changes
> are necessary they should be textually small.  This will make keeping
> us up to date with qemu upstream much easier.

Yes, I couldn't agree more.

Attached is a new version of the patch which creates a dummy CPU for PV
machines. Now it touches no upstream code any more.

Kevin

[-- Attachment #2: fix-pv-segfault.patch --]
[-- Type: text/x-patch, Size: 1456 bytes --]

>From acd73e1af736e0b0e6fe4af234f828b6a572a841 Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@suse.de>
Date: Wed, 2 Jul 2008 13:23:49 +0200
Subject: [PATCH] qemu-xen: Fix PV segfault

Create a dummy CPU during PV machine initialization. This avoids NULL pointer accesses.

Signed-off-by: Kevin Wolf <kwolf@suse.de>
---
 hw/xen_machine_pv.c |    5 ++++-
 i386-dm/helper2.c   |    4 ++++
 2 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/hw/xen_machine_pv.c b/hw/xen_machine_pv.c
index 0fec0aa..e51addb 100644
--- a/hw/xen_machine_pv.c
+++ b/hw/xen_machine_pv.c
@@ -45,13 +45,16 @@ static void xen_init_pv(ram_addr_t ram_size, int vga_ram_size,
 {
     struct xenfb *xenfb;
     extern int domid;
-
+    CPUState *env;
 
 #ifndef CONFIG_STUBDOM
     /* Initialize tapdisk client */
     init_blktap();
 #endif
 
+    /* Initialize a dummy CPU */
+    env = cpu_init(NULL);
+
     /* Connect to text console */
     if (serial_hds[0]) {
         if (xencons_init(domid, serial_hds[0]) < 0) {
diff --git a/i386-dm/helper2.c b/i386-dm/helper2.c
index 44bed2b..6437c37 100644
--- a/i386-dm/helper2.c
+++ b/i386-dm/helper2.c
@@ -112,6 +112,10 @@ CPUX86State *cpu_x86_init(const char *cpu_model)
         return NULL;
     cpu_exec_init(env);
 
+    /* There is no shared_page for PV, we're done now */
+    if (shared_page == NULL)
+        return env;
+
     /* init various static tables */
     if (!inited) {
         inited = 1;
-- 
1.5.4.5


[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel