[PATCH 0 of 5] PV on HVM Xen

[PATCH 0 of 5] PV on HVM Xen

[Stefano Stabellini]

Hi all,
this is a reduced and rebased version of the patch series I sent
yesterday "enhanced PV on HVM": this series is based on Linux 2.5.32 and
can be applied now, it includes everything but the pirq remapping
related functions that are not ready to be upstreamed at the moment.

Therefore it just achieves the goal of enabling PV devices in Linux
running in a Xen HVM domain, it doesn't allow event channels delivery in
place of interrupts.

The patch series consists of 5 patches, each patch comes with a detailed
description.
In order for this to work we also need a patch for Xen, that is being
worked on as we speak.

Any comment, critic or suggestion is very welcome.

Cheers,

Stefano

Re: [Xen-devel] [PATCH 0 of 5] PV on HVM Xen

[Pasi Kärkkäinen]

On Wed, Mar 10, 2010 at 03:46:54PM +0000, Stefano Stabellini wrote:
> Hi all,
> this is a reduced and rebased version of the patch series I sent
> yesterday "enhanced PV on HVM": this series is based on Linux 2.5.32 and
> can be applied now, it includes everything but the pirq remapping
> related functions that are not ready to be upstreamed at the moment.
> 
> Therefore it just achieves the goal of enabling PV devices in Linux
> running in a Xen HVM domain, it doesn't allow event channels delivery in
> place of interrupts.
> 
> The patch series consists of 5 patches, each patch comes with a detailed
> description.
> In order for this to work we also need a patch for Xen, that is being
> worked on as we speak.
>

Hmm.. is it possible to get PV-on-HVM drivers working on older Xen versions,
that don't have additional patches? 

Just like the unmodified_drivers works for 2.6.18 and 2.6.27.

-- Pasi

Re: [Xen-devel] [PATCH 0 of 5] PV on HVM Xen

[Stefano Stabellini]

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

On Wed, 10 Mar 2010, Pasi Kärkkäinen wrote:
> On Wed, Mar 10, 2010 at 03:46:54PM +0000, Stefano Stabellini wrote:
> > Hi all,
> > this is a reduced and rebased version of the patch series I sent
> > yesterday "enhanced PV on HVM": this series is based on Linux 2.5.32 and
> > can be applied now, it includes everything but the pirq remapping
> > related functions that are not ready to be upstreamed at the moment.
> > 
> > Therefore it just achieves the goal of enabling PV devices in Linux
> > running in a Xen HVM domain, it doesn't allow event channels delivery in
> > place of interrupts.
> > 
> > The patch series consists of 5 patches, each patch comes with a detailed
> > description.
> > In order for this to work we also need a patch for Xen, that is being
> > worked on as we speak.
> >
> 
> Hmm.. is it possible to get PV-on-HVM drivers working on older Xen versions,
> that don't have additional patches? 
> 
> Just like the unmodified_drivers works for 2.6.18 and 2.6.27.
> 

The problem is that the old unmodified_drivers use a normal interrupt
from the xen pci platform device to receive event channels, and this
doesn't play well with the idea of remapping all the interrupts with
event channels, that is one if the main goals of the previous "enhanced"
patch series.
For this reason the new PV on HVM patch series sets up a new vector
based callback mechanism.

However it might be still possible to provide an additional, backward
compatible, delivery mechanism in case the new one fails.
I'll try to explore this option.

Re: [Xen-devel] [PATCH 0 of 5] PV on HVM Xen

[Jeremy Fitzhardinge]

On 03/10/2010 09:55 AM, Stefano Stabellini wrote:
> However it might be still possible to provide an additional, backward
> compatible, delivery mechanism in case the new one fails.
> I'll try to explore this option.
>    

Yes, please.  A lot of the interest in pv drivers for hvm comes from 
people running long-deployed Xen systems based on RHEL, etc.

     J

Re: [Xen-devel] [PATCH 0 of 5] PV on HVM Xen

[Sheng Yang]

On Wednesday 10 March 2010 23:46:54 Stefano Stabellini wrote:
> Hi all,

Stefano,

And next time when you send out the patch, please be more respect to my work.

You dropped all the original author(me) of patchset, and only add a sign-off 
for me. If you don't aware the difference, here is a snippet of 
linux/Documentation/SummittingPatches

532 The "from" line must be the very first line in the message body,
533 and has the form:
534
535         From: Original Author <author@example.com>
536
537 The "from" line specifies who will be credited as the author of the
538 patch in the permanent changelog.  If the "from" line is missing,
539 then the "From:" line from the email header will be used to determine
540 the patch author in the changelog.

As you see, the first two kernel patches of mine contained file from Jeremy 
and Keir, and I add the "From" for them properly to respect their works.

Another thing is, you were keeping using my old patches as your base, while I 
was working with the reviewers to update the patch quickly. I don't think 
that's a kind of respect to both reviewers' and my work. You would duplicate 
reviewer's effect, especially you always repost the whole patch(and drop my 
authorship) rather than the different part. I've split patches in order to 
provide a code base for further development, but you complete ignored them and 
keeping post the whole patchset based on my old patches.

Please be more professional. Thanks.

-- 
regards
Yang, Sheng

> this is a reduced and rebased version of the patch series I sent
> yesterday "enhanced PV on HVM": this series is based on Linux 2.5.32 and
> can be applied now, it includes everything but the pirq remapping
> related functions that are not ready to be upstreamed at the moment.
> 
> Therefore it just achieves the goal of enabling PV devices in Linux
> running in a Xen HVM domain, it doesn't allow event channels delivery in
> place of interrupts.
> 
> The patch series consists of 5 patches, each patch comes with a detailed
> description.
> In order for this to work we also need a patch for Xen, that is being
> worked on as we speak.
> 
> Any comment, critic or suggestion is very welcome.
> 
> Cheers,
> 
> Stefano
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
> 

Re: [PATCH 0 of 5] PV on HVM Xen

[Sheng Yang]

On Wednesday 10 March 2010 23:46:54 Stefano Stabellini wrote:
> Hi all,

Stefano,

And next time when you send out the patch, please be more respect to my work.

You dropped all the original author(me) of patchset, and only add a sign-off 
for me. If you don't aware the difference, here is a snippet of 
linux/Documentation/SummittingPatches

532 The "from" line must be the very first line in the message body,
533 and has the form:
534
535         From: Original Author <author@example.com>
536
537 The "from" line specifies who will be credited as the author of the
538 patch in the permanent changelog.  If the "from" line is missing,
539 then the "From:" line from the email header will be used to determine
540 the patch author in the changelog.

As you see, the first two kernel patches of mine contained file from Jeremy 
and Keir, and I add the "From" for them properly to respect their works.

Another thing is, you were keeping using my old patches as your base, while I 
was working with the reviewers to update the patch quickly. I don't think 
that's a kind of respect to both reviewers' and my work. You would duplicate 
reviewer's effect, especially you always repost the whole patch(and drop my 
authorship) rather than the different part. I've split patches in order to 
provide a code base for further development, but you complete ignored them and 
keeping post the whole patchset based on my old patches.

Please be more professional. Thanks.

-- 
regards
Yang, Sheng

> this is a reduced and rebased version of the patch series I sent
> yesterday "enhanced PV on HVM": this series is based on Linux 2.5.32 and
> can be applied now, it includes everything but the pirq remapping
> related functions that are not ready to be upstreamed at the moment.
> 
> Therefore it just achieves the goal of enabling PV devices in Linux
> running in a Xen HVM domain, it doesn't allow event channels delivery in
> place of interrupts.
> 
> The patch series consists of 5 patches, each patch comes with a detailed
> description.
> In order for this to work we also need a patch for Xen, that is being
> worked on as we speak.
> 
> Any comment, critic or suggestion is very welcome.
> 
> Cheers,
> 
> Stefano
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
> 

Re: [Xen-devel] [PATCH 0 of 5] PV on HVM Xen

[Stefano Stabellini]

On Fri, 12 Mar 2010, Sheng Yang wrote:
> On Wednesday 10 March 2010 23:46:54 Stefano Stabellini wrote:
> > Hi all,
> 
> Stefano,
> 
> And next time when you send out the patch, please be more respect to my work.
> 
> You dropped all the original author(me) of patchset, and only add a sign-off 
> for me. If you don't aware the difference, here is a snippet of 
> linux/Documentation/SummittingPatches
> 

I am truly sorry and apologise for it, I would never want to give you
the impression of being disrespectful of you and your work.
If you pay attention I manually wrote in the comments of all the past
versions of the patches that you were the original author, this time I
just forgot.
I guess it is really the time I start using git-send-email :)

Your work has been really important for my series and you deserve the
credit for it independently from which patch series gets applied.


> Another thing is, you were keeping using my old patches as your base, while I 
> was working with the reviewers to update the patch quickly. I don't think 
> that's a kind of respect to both reviewers' and my work. You would duplicate 
> reviewer's effect, especially you always repost the whole patch(and drop my 
> authorship) rather than the different part. I've split patches in order to 
> provide a code base for further development, but you complete ignored them and 
> keeping post the whole patchset based on my old patches.
> 

I don't keep using your old patches as a base but I manually rebase over
the most recent patch series you sent every time.
Obviously it is not a perfect system and sometimes I can miss something,
this is why at the beginning I asked you to work together on the same
tree: I wanted to avoid exactly this sort of issues.

My intentions are true so my proposal of working on a common tree is
still valid, just let me know when you are interested.

Re: [PATCH 0 of 5] PV on HVM Xen

[Stefano Stabellini]

On Fri, 12 Mar 2010, Stefano Stabellini wrote:
> My intentions are true so my proposal of working on a common tree is
> still valid, just let me know when you are interested.
> 

The last versions of our patch series are quite similar, I think at this
point we can really merge them into one.
If you take the time to read the last version of my patch series I
think you'll be able to see it by yourself (missing From: line aside,
again sorry for that).
I looked at the last version of yours and I listed the changes I would
like to be made on top of it, if you and Jeremy agree:


PATCH 1
fine as it is;

PATCH 2
fine as it is;

PATCH 3
I would remove it altogether because I would like to make pv drivers
work on HVM, but considering that at the moment they wouldn't work,
it makes sense to apply it for now;

PATCH 4
I would remove HVMOP_enable_pv, xen_hvm_pv_clock_enabled and the
pvclock for the moment;

PATCH 5
I would change the entry point because I think the one I use in my
patch series is less controversial and probably easier to get accepted
upstream: look at the first part of my second patch;

PATCH 6
I would remove this patch and talk about pv clocksource again when we
do the interrupt remapping work.

Jeremy, what do you think about it?
If we agree on these few basic patches, I'll wait for Jeremy to apply
them in a topic branch and then I'll send my changes on top of them,
adding PV on HVM support (I mean the xen pci platform device driver,
blkfront and netfront) and everybody will be happy.

After this is done we can calmly discuss about how to proceed with the
interrupt remapping stuff.

Re: [PATCH 0 of 5] PV on HVM Xen

[Sheng Yang]

On Saturday 13 March 2010 00:00:42 Stefano Stabellini wrote:
> On Fri, 12 Mar 2010, Stefano Stabellini wrote:
> > My intentions are true so my proposal of working on a common tree is
> > still valid, just let me know when you are interested.
> 
> The last versions of our patch series are quite similar, I think at this
> point we can really merge them into one.
> If you take the time to read the last version of my patch series I
> think you'll be able to see it by yourself (missing From: line aside,
> again sorry for that).
> I looked at the last version of yours and I listed the changes I would
> like to be made on top of it, if you and Jeremy agree:
> 
> 
> PATCH 1
> fine as it is;
> 
> PATCH 2
> fine as it is;
> 
> PATCH 3
> I would remove it altogether because I would like to make pv drivers
> work on HVM, but considering that at the moment they wouldn't work,
> it makes sense to apply it for now;
> 
> PATCH 4
> I would remove HVMOP_enable_pv, xen_hvm_pv_clock_enabled and the
> pvclock for the moment;

See my comments below.

> PATCH 5
> I would change the entry point because I think the one I use in my
> patch series is less controversial and probably easier to get accepted
> upstream: look at the first part of my second patch;

My currently evtchn mapping implementation require disable_acpi=1, which is 
the same as pv guest(and I would look into your implement to see if it's still 
needed), so you can't do it after acpi related initialization. Anyway, I don't 
think the position would be a issue for upstream. HV are picking positions 
according to their requirement, and there are already sparse vm enabling codes 
in setup_arch().

> PATCH 6
> I would remove this patch and talk about pv clocksource again when we
> do the interrupt remapping work.

What's the reason to remove pv clocksource?

In fact, after Jeremy's reminder, I found clocksource and clockevent can be 
decoupled like I demonstrated in my code. So PV clocksource have nothing to do 
with evtchn mapping for HVM(I don't like to call it interrupt remapping 
because that reminder me of a hardware feature...). So I don't understand why 
to remove it.

> Jeremy, what do you think about it?
> If we agree on these few basic patches, I'll wait for Jeremy to apply
> them in a topic branch and then I'll send my changes on top of them,
> adding PV on HVM support (I mean the xen pci platform device driver,
> blkfront and netfront) and everybody will be happy.

I am fine with PCI platform device drivers, if they can work before evtchn 
mapping is ready.
 
> After this is done we can calmly discuss about how to proceed with the
> interrupt remapping stuff.

OK.

-- 
regards
Yang, Sheng

Re: [PATCH 0 of 5] PV on HVM Xen

[Sheng Yang]

On Monday 15 March 2010 12:05:29 Sheng Yang wrote:
> On Saturday 13 March 2010 00:00:42 Stefano Stabellini wrote:
> > On Fri, 12 Mar 2010, Stefano Stabellini wrote:
> > > My intentions are true so my proposal of working on a common tree is
> > > still valid, just let me know when you are interested.
> >
> > The last versions of our patch series are quite similar, I think at this
> > point we can really merge them into one.
> > If you take the time to read the last version of my patch series I
> > think you'll be able to see it by yourself (missing From: line aside,
> > again sorry for that).
> > I looked at the last version of yours and I listed the changes I would
> > like to be made on top of it, if you and Jeremy agree:
> >
> >
> > PATCH 1
> > fine as it is;
> >
> > PATCH 2
> > fine as it is;
> >
> > PATCH 3
> > I would remove it altogether because I would like to make pv drivers
> > work on HVM, but considering that at the moment they wouldn't work,
> > it makes sense to apply it for now;
> >
> > PATCH 4
> > I would remove HVMOP_enable_pv, xen_hvm_pv_clock_enabled and the
> > pvclock for the moment;
> 
> See my comments below.
> 
> > PATCH 5
> > I would change the entry point because I think the one I use in my
> > patch series is less controversial and probably easier to get accepted
> > upstream: look at the first part of my second patch;
> 
> My currently evtchn mapping implementation require disable_acpi=1, which is
> the same as pv guest(and I would look into your implement to see if it's
>  still needed), so you can't do it after acpi related initialization.
>  Anyway, I don't think the position would be a issue for upstream. HV are
>  picking positions according to their requirement, and there are already
>  sparse vm enabling codes in setup_arch().

Hi Stefano

I've checked your patches. And one point puzzled me(I am looking into pv_ops 
dom0 code): seems if it depends on PIRQ, it would still require to do EOI(then 
result in vmexit) for edge triggered interrupt? (through xen_pirq_chip->end). 
And unmask is still a heavy work required vmexit?(seems it need twice vmexit, 
even heavier than current HVM solution)

-- 
regards
Yang, Sheng

Re: [PATCH 0 of 5] PV on HVM Xen

[Stefano Stabellini]

On Mon, 15 Mar 2010, Sheng Yang wrote:
> Hi Stefano
> 
> I've checked your patches. And one point puzzled me(I am looking into pv_ops 
> dom0 code): seems if it depends on PIRQ, it would still require to do EOI(then 
> result in vmexit) for edge triggered interrupt? (through xen_pirq_chip->end). 
> And unmask is still a heavy work required vmexit?(seems it need twice vmexit, 
> even heavier than current HVM solution)
> 
 
Only when pirq_needs_eoi(irq) returns true, and this happens when
PIRQ_NEEDS_EOI is set.
In my patch series I am making sure PIRQ_NEEDS_EOI is never set for any
pirq on HVM.

Re: [PATCH 0 of 5] PV on HVM Xen

[Sheng Yang]

On Monday 15 March 2010 20:22:23 Stefano Stabellini wrote:
> On Mon, 15 Mar 2010, Sheng Yang wrote:
> > Hi Stefano
> >
> > I've checked your patches. And one point puzzled me(I am looking into
> > pv_ops dom0 code): seems if it depends on PIRQ, it would still require to
> > do EOI(then result in vmexit) for edge triggered interrupt? (through
> > xen_pirq_chip->end). And unmask is still a heavy work required
> > vmexit?(seems it need twice vmexit, even heavier than current HVM
> > solution)
> 
> Only when pirq_needs_eoi(irq) returns true, and this happens when
> PIRQ_NEEDS_EOI is set.
> In my patch series I am making sure PIRQ_NEEDS_EOI is never set for any
> pirq on HVM.
> 
OK.

I would like to work on the PIRQ porting upstream Linux(I would try ACPI but 
not sure if it would make code much more complex, e.g. cpu hotplug, PCI 
hotplug... A simple version would be just using PIRQ instead of VIRQ in my 
patches, but PIRQ is flexible, I would see if I can do more with it). And we 
may not get a version exactly the same as pv_ops dom0 code in the end... I 
would try to make them similar and make the HVM part small enough, then reduce 
Jeremy's maintain effort.

-- 
regards
Yang, Sheng

Re: [PATCH 0 of 5] PV on HVM Xen

[Konrad Rzeszutek Wilk]

On Wed, Mar 17, 2010 at 05:38:37PM +0800, Sheng Yang wrote:
> On Monday 15 March 2010 20:22:23 Stefano Stabellini wrote:
> > On Mon, 15 Mar 2010, Sheng Yang wrote:
> > > Hi Stefano
> > >
> > > I've checked your patches. And one point puzzled me(I am looking into
> > > pv_ops dom0 code): seems if it depends on PIRQ, it would still require to
> > > do EOI(then result in vmexit) for edge triggered interrupt? (through
> > > xen_pirq_chip->end). And unmask is still a heavy work required
> > > vmexit?(seems it need twice vmexit, even heavier than current HVM
> > > solution)
> > 
> > Only when pirq_needs_eoi(irq) returns true, and this happens when
> > PIRQ_NEEDS_EOI is set.
> > In my patch series I am making sure PIRQ_NEEDS_EOI is never set for any
> > pirq on HVM.
> > 
> OK.
> 
> I would like to work on the PIRQ porting upstream Linux(I would try ACPI but 
> not sure if it would make code much more complex, e.g. cpu hotplug, PCI 
> hotplug... A simple version would be just using PIRQ instead of VIRQ in my 
> patches, but PIRQ is flexible, I would see if I can do more with it). And we 
> may not get a version exactly the same as pv_ops dom0 code in the end... I 

That is OK. There is no problems in ripping out the dom0 code and
building on top of your code.

Re: [PATCH 0 of 5] PV on HVM Xen

[Stefano Stabellini]

On Wed, 17 Mar 2010, Sheng Yang wrote:
> I would like to work on the PIRQ porting upstream Linux

I am glad to hear that!


> A simple version would be just using PIRQ instead of VIRQ in my 
> patches, but PIRQ is flexible, I would see if I can do more with it

After the first part of your series gets applied, I'll rebase my pirq
remapping patches on top of it, then you can tell me what's wrong so I
can change it. I am very open to critics or suggestions.
I would also prefer that you take my pirq remapping patches, you make
any change you like, then you send your work to the list, rather than
reinventing the wheel.
I value your contributions so I would be happy if you could work on my
series, that even though is working still misses MSI support and I am
sure you'll be able to make many other important improvements.


> And we may not get a version exactly the same as pv_ops dom0 code in
> the end... I would try to make them similar and make the HVM part
> small enough, then reduce Jeremy's maintain effort.
> 

As pointed out before by Jeremy and Konrad, the best starting point is
probably Konrad's pv/pcifront-2.6.31 tree: it contains most of the pirq
stuff, ready to be upstreamed.
AFAICT the only things required to make pirq mappings work (as in
my series) that are missing are:

- xen_register_gsi

- xen_setup_pirqs

- the xen_register_gsi hook in acpi_register_gsi

the first two should be easy to port because they don't require any
change but the last one definitely needs modifications in order to be
accepted upstream.
I didn't include MSI support because is not required, but that is
another area that needs changes.

Re: [PATCH 0 of 5] PV on HVM Xen

[Jeremy Fitzhardinge]

On 03/17/2010 08:21 AM, Stefano Stabellini wrote:
> As pointed out before by Jeremy and Konrad, the best starting point is
> probably Konrad's pv/pcifront-2.6.31 tree: it contains most of the pirq
> stuff, ready to be upstreamed.
> AFAICT the only things required to make pirq mappings work (as in
> my series) that are missing are:
>
> - xen_register_gsi
>
> - xen_setup_pirqs
>
> - the xen_register_gsi hook in acpi_register_gsi
>
> the first two should be easy to port because they don't require any
> change but the last one definitely needs modifications in order to be
> accepted upstream.
>    

Have a look at 108bfa4b9029bd0f9775319900f82eb9749f5954 and 
78c07daac0d9e267b5c23a3e9fbd3c7697f4ef44 in 
xen/dom0/"new"-interrupt-routing.  I think this will be a plausible 
thing to upstream.

> I didn't include MSI support because is not required, but that is
> another area that needs changes.
>    

Xiantao has some interesting ideas for this.

     J

Re: [PATCH 0 of 5] PV on HVM Xen

[Sheng Yang]

On Thursday 18 March 2010 00:13:00 Jeremy Fitzhardinge wrote:
> On 03/17/2010 08:21 AM, Stefano Stabellini wrote:
> > As pointed out before by Jeremy and Konrad, the best starting point is
> > probably Konrad's pv/pcifront-2.6.31 tree: it contains most of the pirq
> > stuff, ready to be upstreamed.
> > AFAICT the only things required to make pirq mappings work (as in
> > my series) that are missing are:
> >
> > - xen_register_gsi
> >
> > - xen_setup_pirqs
> >
> > - the xen_register_gsi hook in acpi_register_gsi
> >
> > the first two should be easy to port because they don't require any
> > change but the last one definitely needs modifications in order to be
> > accepted upstream.
> 
> Have a look at 108bfa4b9029bd0f9775319900f82eb9749f5954 and
> 78c07daac0d9e267b5c23a3e9fbd3c7697f4ef44 in
> xen/dom0/"new"-interrupt-routing.  I think this will be a plausible
> thing to upstream.

Would look into it.
> 
> > I didn't include MSI support because is not required, but that is
> > another area that needs changes.
> 
> Xiantao has some interesting ideas for this.
> 
Xiantao and I have discussed on this for a month... Basically we have got two 
approaches now, but we can't reach an agreement... I would work on it after 
current hybrid thing settled down. Of course, we want MSI support benefit  
pv_ops dom0 as well as hybrid.

-- 
regards
Yang, Sheng

Re: [PATCH 0 of 5] PV on HVM Xen

[Jeremy Fitzhardinge]

On 03/17/2010 06:30 PM, Sheng Yang wrote:
>> Xiantao has some interesting ideas for this.
>>
>>      
> Xiantao and I have discussed on this for a month... Basically we have got two
> approaches now, but we can't reach an agreement... I would work on it after
> current hybrid thing settled down. Of course, we want MSI support benefit
> pv_ops dom0 as well as hybrid.
>    

Xiantao's proposal of a new top-level MSI API for the kernel looks 
pretty clean, and I think it has a reasonable chance of being accepted 
upstream.

What's your proposal?

Thanks,
     J

MSI proposal and work transfer...(was: Re: [PATCH 0 of 5] PV on HVM Xen)

[Sheng Yang]

On Saturday 20 March 2010 04:38:23 Jeremy Fitzhardinge wrote:
> On 03/17/2010 06:30 PM, Sheng Yang wrote:
> >> Xiantao has some interesting ideas for this.
> >
> > Xiantao and I have discussed on this for a month... Basically we have got
> > two approaches now, but we can't reach an agreement... I would work on it
> > after current hybrid thing settled down. Of course, we want MSI support
> > benefit pv_ops dom0 as well as hybrid.
> 
> Xiantao's proposal of a new top-level MSI API for the kernel looks
> pretty clean, and I think it has a reasonable chance of being accepted
> upstream.
> 
> What's your proposal?

My proposal is to do these in the lower level compared to Xiantao's proposal, 
because I don't think touch PCI subsystem is a good idea for upstream check 
in.

We can take advantage of the fact that MSI data/address formating can be 
defined by each architecture, and at the same time, trap the accessing in the 
Xen, passthrough the most PCI configuration space accessing but intercepted 
MSI data/address accessing, so that we can write the real data to the hardware 
when guest try to write Xen specific MSI data/address format.

The hook position would be arch_setup_msi_irqs(), which would create the 
vector and write the x86 LAPIC specific format to MSI data/address. By this 
way, we can limit the impact inside x86 arch. We would write the information 
contained evtchn/PIRQ in it, so that we can setup the mapping. And this same 
point works for MSI and MSI-X, and S3 wouldn't be a issue if we trap the 
accessing.

I've used this way on MSI support for hybrid guest several months ago, and the 
kernel code impact is very small(of course, at that time, the intercept is in 
QEmu rather than Xen).

I think it's not easy for a hook in the whole PCI subsystem to be accepted by 
upstream Linux. Anyway, it's my estimation. 

Another thing is, due to some other task assignment to me days ago, I am 
afraid I have to stop my working on PV extension of HVM guest, as well as MSI 
work which we considered as a part of PV interrupt delivery mechanism for 
Hybrid. You know, it's really a hard decision to me, but I have no choice...

So I would like to transfer the current work to someone who interested in it. 
The next step is somehow clear. We would have a PV clocksource for HVM, as 
well as PIRQ mapped irqchip to speed up interrupt delivery. 

Stefano, would you like help to take my work and continue it? I think no one 
is more familiar with these discussion and code than you in the community. The 
final target is still upstream Linux I hope... 

Thanks!

-- 
regards
Yang, Sheng

Re: MSI proposal and work transfer...(was: Re: [PATCH 0 of 5] PV on HVM Xen)

[Jeremy Fitzhardinge]

On 03/21/2010 11:26 PM, Sheng Yang wrote:
> On Saturday 20 March 2010 04:38:23 Jeremy Fitzhardinge wrote:
>    
>> On 03/17/2010 06:30 PM, Sheng Yang wrote:
>>      
>>>> Xiantao has some interesting ideas for this.
>>>>          
>>> Xiantao and I have discussed on this for a month... Basically we have got
>>> two approaches now, but we can't reach an agreement... I would work on it
>>> after current hybrid thing settled down. Of course, we want MSI support
>>> benefit pv_ops dom0 as well as hybrid.
>>>        
>> Xiantao's proposal of a new top-level MSI API for the kernel looks
>> pretty clean, and I think it has a reasonable chance of being accepted
>> upstream.
>>
>> What's your proposal?
>>      
> My proposal is to do these in the lower level compared to Xiantao's proposal,
> because I don't think touch PCI subsystem is a good idea for upstream check
> in.
>
> We can take advantage of the fact that MSI data/address formating can be
> defined by each architecture, and at the same time, trap the accessing in the
> Xen, passthrough the most PCI configuration space accessing but intercepted
> MSI data/address accessing, so that we can write the real data to the hardware
> when guest try to write Xen specific MSI data/address format.
>
> The hook position would be arch_setup_msi_irqs(), which would create the
> vector and write the x86 LAPIC specific format to MSI data/address. By this
> way, we can limit the impact inside x86 arch. We would write the information
> contained evtchn/PIRQ in it, so that we can setup the mapping. And this same
> point works for MSI and MSI-X, and S3 wouldn't be a issue if we trap the
> accessing.
>    

I would be interested in seeing what the patches look like for this.

But to be quite honest, it could well be easier to introduce a new nice, 
clean, self-contained and consistent API at the appropriate level of 
abstraction rather than trying to shoe-horn one into the arch/x86 
layer.  It sounds like your proposal may well save some general kernel 
code changes, but at the expense of being quite complex under the covers.

> Another thing is, due to some other task assignment to me days ago, I am
> afraid I have to stop my working on PV extension of HVM guest, as well as MSI
> work which we considered as a part of PV interrupt delivery mechanism for
> Hybrid. You know, it's really a hard decision to me, but I have no choice...
>
> So I would like to transfer the current work to someone who interested in it.
> The next step is somehow clear. We would have a PV clocksource for HVM, as
> well as PIRQ mapped irqchip to speed up interrupt delivery.
>
> Stefano, would you like help to take my work and continue it? I think no one
> is more familiar with these discussion and code than you in the community. The
> final target is still upstream Linux I hope...
>    

That's unfortunate; things seem to have been progressing quite well, and 
I'd really like to get something ready to commit (and possibly upstream) 
soon.  Stefano, will you be able to finish things off?

Thanks,
     J

Re: MSI proposal and work transfer...(was: Re: [PATCH 0 of 5] PV on HVM Xen)

[Sheng Yang]

On Wednesday 24 March 2010 04:47:58 Jeremy Fitzhardinge wrote:
> On 03/21/2010 11:26 PM, Sheng Yang wrote:
> > On Saturday 20 March 2010 04:38:23 Jeremy Fitzhardinge wrote:
> >> On 03/17/2010 06:30 PM, Sheng Yang wrote:
> >>>> Xiantao has some interesting ideas for this.
> >>>
> >>> Xiantao and I have discussed on this for a month... Basically we have
> >>> got two approaches now, but we can't reach an agreement... I would work
> >>> on it after current hybrid thing settled down. Of course, we want MSI
> >>> support benefit pv_ops dom0 as well as hybrid.
> >>
> >> Xiantao's proposal of a new top-level MSI API for the kernel looks
> >> pretty clean, and I think it has a reasonable chance of being accepted
> >> upstream.
> >>
> >> What's your proposal?
> >
> > My proposal is to do these in the lower level compared to Xiantao's
> > proposal, because I don't think touch PCI subsystem is a good idea for
> > upstream check in.
> >
> > We can take advantage of the fact that MSI data/address formating can be
> > defined by each architecture, and at the same time, trap the accessing in
> > the Xen, passthrough the most PCI configuration space accessing but
> > intercepted MSI data/address accessing, so that we can write the real
> > data to the hardware when guest try to write Xen specific MSI
> > data/address format.
> >
> > The hook position would be arch_setup_msi_irqs(), which would create the
> > vector and write the x86 LAPIC specific format to MSI data/address. By
> > this way, we can limit the impact inside x86 arch. We would write the
> > information contained evtchn/PIRQ in it, so that we can setup the
> > mapping. And this same point works for MSI and MSI-X, and S3 wouldn't be
> > a issue if we trap the accessing.
> 
> I would be interested in seeing what the patches look like for this.
> 
> But to be quite honest, it could well be easier to introduce a new nice,
> clean, self-contained and consistent API at the appropriate level of
> abstraction rather than trying to shoe-horn one into the arch/x86
> layer.  It sounds like your proposal may well save some general kernel
> code changes, but at the expense of being quite complex under the covers.

I think the key for checking in is small footprint and only necessary changes 
allowed. PCI spec is there, define what's is MSI and MSI-X, and how should we 
deal with it. MSI hook is easy for Xen, but not easy for Linux upstream I 
think. 

Anyway, it's up to you...

-- 
regards
Yang, Sheng
 
> > Another thing is, due to some other task assignment to me days ago, I am
> > afraid I have to stop my working on PV extension of HVM guest, as well as
> > MSI work which we considered as a part of PV interrupt delivery mechanism
> > for Hybrid. You know, it's really a hard decision to me, but I have no
> > choice...
> >
> > So I would like to transfer the current work to someone who interested in
> > it. The next step is somehow clear. We would have a PV clocksource for
> > HVM, as well as PIRQ mapped irqchip to speed up interrupt delivery.
> >
> > Stefano, would you like help to take my work and continue it? I think no
> > one is more familiar with these discussion and code than you in the
> > community. The final target is still upstream Linux I hope...
> 
> That's unfortunate; things seem to have been progressing quite well, and
> I'd really like to get something ready to commit (and possibly upstream)
> soon.  Stefano, will you be able to finish things off?
> 
> Thanks,
>      J
> 

Re: MSI proposal and work transfer...(was: Re: [PATCH 0 of 5] PV on HVM Xen)

[Stefano Stabellini]

On Mon, 22 Mar 2010, Sheng Yang wrote:
> Another thing is, due to some other task assignment to me days ago, I am 
> afraid I have to stop my working on PV extension of HVM guest, as well as MSI 
> work which we considered as a part of PV interrupt delivery mechanism for 
> Hybrid. You know, it's really a hard decision to me, but I have no choice...
> 

I am sorry to hear that.

> So I would like to transfer the current work to someone who interested in it. 
> The next step is somehow clear. We would have a PV clocksource for HVM, as 
> well as PIRQ mapped irqchip to speed up interrupt delivery. 
> 
> Stefano, would you like help to take my work and continue it? I think no one 
> is more familiar with these discussion and code than you in the community.

Sure, I would be happy to help finish your work.


> The 
> final target is still upstream Linux I hope... 
> 

Yes, my target is upstream Linux as well.

Re: MSI proposal and work transfer...(was: Re: [PATCH 0 of 5] PV on HVM Xen)

[Sheng Yang]

On Wednesday 24 March 2010 07:16:10 Stefano Stabellini wrote:
> On Mon, 22 Mar 2010, Sheng Yang wrote:
> > Another thing is, due to some other task assignment to me days ago, I am
> > afraid I have to stop my working on PV extension of HVM guest, as well as
> > MSI work which we considered as a part of PV interrupt delivery mechanism
> > for Hybrid. You know, it's really a hard decision to me, but I have no
> > choice...
> 
> I am sorry to hear that.
> 
> > So I would like to transfer the current work to someone who interested in
> > it. The next step is somehow clear. We would have a PV clocksource for
> > HVM, as well as PIRQ mapped irqchip to speed up interrupt delivery.
> >
> > Stefano, would you like help to take my work and continue it? I think no
> > one is more familiar with these discussion and code than you in the
> > community.
> 
> Sure, I would be happy to help finish your work.
> 
> > The
> > final target is still upstream Linux I hope...
> 
> Yes, my target is upstream Linux as well.

Thanks Stefano! I wish we can get the code into upstream Linux soon, also one 
step for pv_ops dom0 to enter upstream. :)

-- 
regards
Yang, Sheng

Re: [PATCH 0 of 5] PV on HVM Xen

[Sheng Yang]

On Wednesday 17 March 2010 23:21:08 Stefano Stabellini wrote:
> On Wed, 17 Mar 2010, Sheng Yang wrote:
> > I would like to work on the PIRQ porting upstream Linux
> 
> I am glad to hear that!
> 
> > A simple version would be just using PIRQ instead of VIRQ in my
> > patches, but PIRQ is flexible, I would see if I can do more with it
> 
> After the first part of your series gets applied, I'll rebase my pirq
> remapping patches on top of it, then you can tell me what's wrong so I
> can change it. I am very open to critics or suggestions.
> I would also prefer that you take my pirq remapping patches, you make
> any change you like, then you send your work to the list, rather than
> reinventing the wheel.

Yeah, I'd like to take your patch and modify it as much as possible, which 
would be more efficient. Of course, with your signed-off hold. :)

-- 
regards
Yang, Sheng

> I value your contributions so I would be happy if you could work on my
> series, that even though is working still misses MSI support and I am
> sure you'll be able to make many other important improvements.
> 
> > And we may not get a version exactly the same as pv_ops dom0 code in
> > the end... I would try to make them similar and make the HVM part
> > small enough, then reduce Jeremy's maintain effort.
> 
> As pointed out before by Jeremy and Konrad, the best starting point is
> probably Konrad's pv/pcifront-2.6.31 tree: it contains most of the pirq
> stuff, ready to be upstreamed.
> AFAICT the only things required to make pirq mappings work (as in
> my series) that are missing are:
> 
> - xen_register_gsi
> 
> - xen_setup_pirqs
> 
> - the xen_register_gsi hook in acpi_register_gsi
> 
> the first two should be easy to port because they don't require any
> change but the last one definitely needs modifications in order to be
> accepted upstream.
> I didn't include MSI support because is not required, but that is
> another area that needs changes.
> 

Re: [PATCH 0 of 5] PV on HVM Xen

[Jeremy Fitzhardinge]

On 03/17/2010 07:19 PM, Sheng Yang wrote:
> Yeah, I'd like to take your patch and modify it as much as possible, which
> would be more efficient. Of course, with your signed-off hold. :)
>    

It would be best to publish your work as a delta patch against 
Stefano's, at least for development.  Once we're all happy with the 
result, we can see if it makes sense to fold the patches together or 
keep them distinct (keeping them separate makes questions of crediting 
the work much easier to deal with).

     J

Re: [PATCH 0 of 5] PV on HVM Xen

[Jeremy Fitzhardinge]

On 03/17/2010 02:38 AM, Sheng Yang wrote:
> I
> would try to make them similar and make the HVM part small enough, then reduce
> Jeremy's maintain effort.
>    

Sounds good to me ;)

     J

Re: [PATCH 0 of 5] PV on HVM Xen

[Stefano Stabellini]

On Mon, 15 Mar 2010, Sheng Yang wrote:
> My currently evtchn mapping implementation require disable_acpi=1, which is 
> the same as pv guest(and I would look into your implement to see if it's still 
> needed), so you can't do it after acpi related initialization. Anyway, I don't 
> think the position would be a issue for upstream. HV are picking positions 
> according to their requirement, and there are already sparse vm enabling codes 
> in setup_arch().

I see.
I have tested my series with acpi enable, the only problem I had is that
acpi might report more vcpus than actually present on the system so some
hypercalls to enable secondary vcpus might fail.
Is this the reason why you disable acpi?
To be sure you can add an hack to xen_cpu_up to return immediately if
cpu is greater than the number of vcpus in your VM.
If this is the only reason I think it can be fixed.
Also the new "reduced" series you sent doesn't have such limitation
anyway.


> > PATCH 6
> > I would remove this patch and talk about pv clocksource again when we
> > do the interrupt remapping work.
> 
> What's the reason to remove pv clocksource?
> 
> In fact, after Jeremy's reminder, I found clocksource and clockevent can be 
> decoupled like I demonstrated in my code. So PV clocksource have nothing to do 
> with evtchn mapping for HVM(I don't like to call it interrupt remapping 
> because that reminder me of a hardware feature...). So I don't understand why 
> to remove it.
> 

I like your pv clocksource implementation.
The only reason why I would defer the patch is that I don't particularly
like the "enable_pv" hypercall, so I would try to get away without it,
resetting the tsc offset automatically when enabling the VIRQ_TIMER on
an HVM domain.
But everything else is fine: as I said I would just postpone it, I would
not remove it.


> > Jeremy, what do you think about it?
> > If we agree on these few basic patches, I'll wait for Jeremy to apply
> > them in a topic branch and then I'll send my changes on top of them,
> > adding PV on HVM support (I mean the xen pci platform device driver,
> > blkfront and netfront) and everybody will be happy.
> 
> I am fine with PCI platform device drivers, if they can work before evtchn 
> mapping is ready.
>  

yep, no mappings required

Re: [PATCH 0 of 5] PV on HVM Xen

[Jeremy Fitzhardinge]

On 03/15/2010 05:28 AM, Stefano Stabellini wrote:
> I like your pv clocksource implementation.
> The only reason why I would defer the patch is that I don't particularly
> like the "enable_pv" hypercall, so I would try to get away without it,
> resetting the tsc offset automatically when enabling the VIRQ_TIMER on
> an HVM domain.
>    

Ah, so the issue is that if we're using the pvclock, the host and guest 
need to share the same tsc, so we can't deal with any kind of tsc offset?

In that case, I'd prefer to have an explicit "set/remove tsc offset" 
vcpu op rather than making it the implicit side-effect of anything 
else.  In particular, since clock sources and event sources are 
completely distinct, making tsc offset (a clock source thing) affected 
VIRQ_TIMER (and event source thing) seems like a particularly poor idea.

That, or make the pvclock structure the HVM vcpu sees have timing 
parameters which already incorporate the tsc offset.  We've already 
demonstrated that there's no need to have the time info in the real 
shared memory between Xen and the domain (it can be updated via copy 
when needed).

     J

Re: [PATCH 0 of 5] PV on HVM Xen

[Frank van der Linden]

On 03/15/10 05:08 PM, Jeremy Fitzhardinge wrote:
> On 03/15/2010 05:28 AM, Stefano Stabellini wrote:
>> I like your pv clocksource implementation.
>> The only reason why I would defer the patch is that I don't particularly
>> like the "enable_pv" hypercall, so I would try to get away without it,
>> resetting the tsc offset automatically when enabling the VIRQ_TIMER on
>> an HVM domain.
>
> Ah, so the issue is that if we're using the pvclock, the host and 
> guest need to share the same tsc, so we can't deal with any kind of 
> tsc offset?
>
> In that case, I'd prefer to have an explicit "set/remove tsc offset" 
> vcpu op rather than making it the implicit side-effect of anything 
> else.  In particular, since clock sources and event sources are 
> completely distinct, making tsc offset (a clock source thing) affected 
> VIRQ_TIMER (and event source thing) seems like a particularly poor idea.
>
> That, or make the pvclock structure the HVM vcpu sees have timing 
> parameters which already incorporate the tsc offset.  We've already 
> demonstrated that there's no need to have the time info in the real 
> shared memory between Xen and the domain (it can be updated via copy 
> when needed).

I'd like to see it done explicitly too. You could use PV timestamps 
without actually using VIRQ_TIMER. It would not be an optimal 
combination, but you could do it. In fact, just today I looked at an old 
patch that I had lying around to do just this for Solaris PV domU.

Also, relying on side-effects makes for bad interfaces.

- Frank

RE: [PATCH 0 of 5] PV on HVM Xen

[Dan Magenheimer]

Sorry I've fallen hopelessly behind on this thread (rope? :-)
so apologies if this has already been addressed:

If the host and guest need to share the same tsc to work
properly, and there are apps in the guest that assume a
monotonically increasing TSC (or even "approximately monotonically
increasing", e.g. can filter out reasonably small differences),
and the VM migrates from a physical machine that's been running
a long time to a recently booted physical machine, what
happens to the app?  (Answer: Prior to 4.0, or with tsc_mode==2
in 4.0, the app fails.)

TSC emulation addresses this issue (which is why it is effectively
the default in Xen 4.0**) but has the side effect that the TSC reads
required for the pvclock algorithm are trapped/emulated.  In this
case, pvclock may not be much faster than alternative
clocksource implementations.

Sheng/Stefano, have you actually measured pvclock recently
and compared it to other alternatives?  I'm wondering how
much of this discussion is moot.

Dan

P.S. Note that TSC emulation in many cases behaves differently
before and after migration, specifically on modern processors
depending on the clock frequency of the source/destination
machines, so please ensure post-migration is measured as this
will be the common case in many virtualized data centers.

** see xen-unstable.hg/docs/misc/tscmode.txt

> -----Original Message-----
> From: Jeremy Fitzhardinge [mailto:jeremy@goop.org]
> Sent: Monday, March 15, 2010 5:09 PM
> To: Stefano Stabellini
> Cc: xen-devel@lists.xensource.com; Sheng Yang
> Subject: Re: [Xen-devel] [PATCH 0 of 5] PV on HVM Xen
> 
> On 03/15/2010 05:28 AM, Stefano Stabellini wrote:
> > I like your pv clocksource implementation.
> > The only reason why I would defer the patch is that I don't
> particularly
> > like the "enable_pv" hypercall, so I would try to get away without
> it,
> > resetting the tsc offset automatically when enabling the VIRQ_TIMER
> on
> > an HVM domain.
> >
> 
> Ah, so the issue is that if we're using the pvclock, the host and guest
> need to share the same tsc, so we can't deal with any kind of tsc
> offset?
> 
> In that case, I'd prefer to have an explicit "set/remove tsc offset"
> vcpu op rather than making it the implicit side-effect of anything
> else.  In particular, since clock sources and event sources are
> completely distinct, making tsc offset (a clock source thing) affected
> VIRQ_TIMER (and event source thing) seems like a particularly poor
> idea.
> 
> That, or make the pvclock structure the HVM vcpu sees have timing
> parameters which already incorporate the tsc offset.  We've already
> demonstrated that there's no need to have the time info in the real
> shared memory between Xen and the domain (it can be updated via copy
> when needed).
> 
>      J
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH 0 of 5] PV on HVM Xen

[Sheng Yang]

On Tuesday 16 March 2010 08:32:22 Dan Magenheimer wrote:
> Sorry I've fallen hopelessly behind on this thread (rope? :-)
> so apologies if this has already been addressed:
> 
> If the host and guest need to share the same tsc to work
> properly, and there are apps in the guest that assume a
> monotonically increasing TSC (or even "approximately monotonically
> increasing", e.g. can filter out reasonably small differences),
> and the VM migrates from a physical machine that's been running
> a long time to a recently booted physical machine, what
> happens to the app?  (Answer: Prior to 4.0, or with tsc_mode==2
> in 4.0, the app fails.)

I haven't checked the live migration issue. But how does PV guest or kvmclock 
deal with it? The same should works here. Even we may got some trouble in some 
condition, we can still fall back to normal clocksource without any 
loss(though we didn't want this thing always happen).
> 
> TSC emulation addresses this issue (which is why it is effectively
> the default in Xen 4.0**) but has the side effect that the TSC reads
> required for the pvclock algorithm are trapped/emulated.  In this
> case, pvclock may not be much faster than alternative
> clocksource implementations.

The hardware emulation is always not the best solution to address timer issue. 
For example, now we have 3 kinds of different timer mode, and we have to let 
user to determine which one is correct. And hope we won't need more timer mode 
in the future. PV is always the best way to handle timer. Take a look at KVM, 
they don't use much PV, but kvmclock is a must.

> > Sheng/Stefano, have you actually measured pvclock recently
> and compared it to other alternatives?  I'm wondering how
> much of this discussion is moot.

If live migration is a issue now, we can address it. But hardware emulation is 
even a issue for non-LM case - guest need to know the timer mode, which is 
impossible to address. It's surely not the preferred method for VM if a PV way 
is available(of course, and works well).

-- 
regards
Yang, Sheng


> Dan
> 
> P.S. Note that TSC emulation in many cases behaves differently
> before and after migration, specifically on modern processors
> depending on the clock frequency of the source/destination
> machines, so please ensure post-migration is measured as this
> will be the common case in many virtualized data centers.
> 
> ** see xen-unstable.hg/docs/misc/tscmode.txt
> 
> > -----Original Message-----
> > From: Jeremy Fitzhardinge [mailto:jeremy@goop.org]
> > Sent: Monday, March 15, 2010 5:09 PM
> > To: Stefano Stabellini
> > Cc: xen-devel@lists.xensource.com; Sheng Yang
> > Subject: Re: [Xen-devel] [PATCH 0 of 5] PV on HVM Xen
> >
> > On 03/15/2010 05:28 AM, Stefano Stabellini wrote:
> > > I like your pv clocksource implementation.
> > > The only reason why I would defer the patch is that I don't
> >
> > particularly
> >
> > > like the "enable_pv" hypercall, so I would try to get away without
> >
> > it,
> >
> > > resetting the tsc offset automatically when enabling the VIRQ_TIMER
> >
> > on
> >
> > > an HVM domain.
> >
> > Ah, so the issue is that if we're using the pvclock, the host and guest
> > need to share the same tsc, so we can't deal with any kind of tsc
> > offset?
> >
> > In that case, I'd prefer to have an explicit "set/remove tsc offset"
> > vcpu op rather than making it the implicit side-effect of anything
> > else.  In particular, since clock sources and event sources are
> > completely distinct, making tsc offset (a clock source thing) affected
> > VIRQ_TIMER (and event source thing) seems like a particularly poor
> > idea.
> >
> > That, or make the pvclock structure the HVM vcpu sees have timing
> > parameters which already incorporate the tsc offset.  We've already
> > demonstrated that there's no need to have the time info in the real
> > shared memory between Xen and the domain (it can be updated via copy
> > when needed).
> >
> >      J
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel
> 

RE: [PATCH 0 of 5] PV on HVM Xen

[Dan Magenheimer]

Hi Sheng --

A few comments:
- with default tsc_mode, PV handles migration fine
  and I think HVM does also.  But that is because,
  in many situations in 4.0, rdtsc is trapped/emulated
- pvclock is not always the best way to handle timer
  (tsc as the clocksource is better in some cases...
  ask VMware)
- because KVM does something doesn't mean it is the
  best way to do it ;-)
- pvclock doesn't eliminate timer_mode cases...
  timer_mode is required for legacy guest OS's
- in case there is any confusion, tsc_mode and timer_mode
  are different options for very different problems

There are many many different cases to consider across
different processors, different guest OS's, different
migration scenarios.  Have you measured any of them
(using 4.0) or are you using old data or simply making
the assumption that the existing pvclock mechanism is
always the fastest?

I am very supportive of virtualization-aware guest OS's,
but let's get some measurements.

Dan

> -----Original Message-----
> From: Sheng Yang [mailto:sheng@linux.intel.com]
> Sent: Tuesday, March 16, 2010 12:09 AM
> To: Dan Magenheimer
> Cc: Jeremy Fitzhardinge; xen-devel@lists.xensource.com; Eddie Dong;
> Frank Van Der Linden; Stefano Stabellini
> Subject: Re: [Xen-devel] [PATCH 0 of 5] PV on HVM Xen
> 
> On Tuesday 16 March 2010 08:32:22 Dan Magenheimer wrote:
> > Sorry I've fallen hopelessly behind on this thread (rope? :-)
> > so apologies if this has already been addressed:
> >
> > If the host and guest need to share the same tsc to work
> > properly, and there are apps in the guest that assume a
> > monotonically increasing TSC (or even "approximately monotonically
> > increasing", e.g. can filter out reasonably small differences),
> > and the VM migrates from a physical machine that's been running
> > a long time to a recently booted physical machine, what
> > happens to the app?  (Answer: Prior to 4.0, or with tsc_mode==2
> > in 4.0, the app fails.)
> 
> I haven't checked the live migration issue. But how does PV guest or
> kvmclock
> deal with it? The same should works here. Even we may got some trouble
> in some
> condition, we can still fall back to normal clocksource without any
> loss(though we didn't want this thing always happen).
> >
> > TSC emulation addresses this issue (which is why it is effectively
> > the default in Xen 4.0**) but has the side effect that the TSC reads
> > required for the pvclock algorithm are trapped/emulated.  In this
> > case, pvclock may not be much faster than alternative
> > clocksource implementations.
> 
> The hardware emulation is always not the best solution to address timer
> issue.
> For example, now we have 3 kinds of different timer mode, and we have
> to let
> user to determine which one is correct. And hope we won't need more
> timer mode
> in the future. PV is always the best way to handle timer. Take a look
> at KVM,
> they don't use much PV, but kvmclock is a must.
> 
> > > Sheng/Stefano, have you actually measured pvclock recently
> > and compared it to other alternatives?  I'm wondering how
> > much of this discussion is moot.
> 
> If live migration is a issue now, we can address it. But hardware
> emulation is
> even a issue for non-LM case - guest need to know the timer mode, which
> is
> impossible to address. It's surely not the preferred method for VM if a
> PV way
> is available(of course, and works well).
> 
> --
> regards
> Yang, Sheng
> 
> 
> > Dan
> >
> > P.S. Note that TSC emulation in many cases behaves differently
> > before and after migration, specifically on modern processors
> > depending on the clock frequency of the source/destination
> > machines, so please ensure post-migration is measured as this
> > will be the common case in many virtualized data centers.
> >
> > ** see xen-unstable.hg/docs/misc/tscmode.txt
> >
> > > -----Original Message-----
> > > From: Jeremy Fitzhardinge [mailto:jeremy@goop.org]
> > > Sent: Monday, March 15, 2010 5:09 PM
> > > To: Stefano Stabellini
> > > Cc: xen-devel@lists.xensource.com; Sheng Yang
> > > Subject: Re: [Xen-devel] [PATCH 0 of 5] PV on HVM Xen
> > >
> > > On 03/15/2010 05:28 AM, Stefano Stabellini wrote:
> > > > I like your pv clocksource implementation.
> > > > The only reason why I would defer the patch is that I don't
> > >
> > > particularly
> > >
> > > > like the "enable_pv" hypercall, so I would try to get away
> without
> > >
> > > it,
> > >
> > > > resetting the tsc offset automatically when enabling the
> VIRQ_TIMER
> > >
> > > on
> > >
> > > > an HVM domain.
> > >
> > > Ah, so the issue is that if we're using the pvclock, the host and
> guest
> > > need to share the same tsc, so we can't deal with any kind of tsc
> > > offset?
> > >
> > > In that case, I'd prefer to have an explicit "set/remove tsc
> offset"
> > > vcpu op rather than making it the implicit side-effect of anything
> > > else.  In particular, since clock sources and event sources are
> > > completely distinct, making tsc offset (a clock source thing)
> affected
> > > VIRQ_TIMER (and event source thing) seems like a particularly poor
> > > idea.
> > >
> > > That, or make the pvclock structure the HVM vcpu sees have timing
> > > parameters which already incorporate the tsc offset.  We've already
> > > demonstrated that there's no need to have the time info in the real
> > > shared memory between Xen and the domain (it can be updated via
> copy
> > > when needed).
> > >
> > >      J
> > >
> > > _______________________________________________
> > > Xen-devel mailing list
> > > Xen-devel@lists.xensource.com
> > > http://lists.xensource.com/xen-devel
> >
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH 0 of 5] PV on HVM Xen

[Stefano Stabellini]

On Mon, 15 Mar 2010, Jeremy Fitzhardinge wrote:
> On 03/15/2010 05:28 AM, Stefano Stabellini wrote:
> > I like your pv clocksource implementation.
> > The only reason why I would defer the patch is that I don't particularly
> > like the "enable_pv" hypercall, so I would try to get away without it,
> > resetting the tsc offset automatically when enabling the VIRQ_TIMER on
> > an HVM domain.
> >    
> 
> Ah, so the issue is that if we're using the pvclock, the host and guest 
> need to share the same tsc, so we can't deal with any kind of tsc offset?
> 
> In that case, I'd prefer to have an explicit "set/remove tsc offset" 
> vcpu op rather than making it the implicit side-effect of anything 
> else.  In particular, since clock sources and event sources are 
> completely distinct, making tsc offset (a clock source thing) affected 
> VIRQ_TIMER (and event source thing) seems like a particularly poor idea.
> 
> That, or make the pvclock structure the HVM vcpu sees have timing 
> parameters which already incorporate the tsc offset.  We've already 
> demonstrated that there's no need to have the time info in the real 
> shared memory between Xen and the domain (it can be updated via copy 
> when needed).
> 
 
OK, you are right: having an explicit "set/remove tsc offset" is
the best solution.
Sheng, are you OK with this?

Re: [PATCH 0 of 5] PV on HVM Xen

[Jeremy Fitzhardinge]

On 03/16/2010 04:07 AM, Stefano Stabellini wrote:
> OK, you are right: having an explicit "set/remove tsc offset" is
> the best solution.
>    

I wonder if we should include scale in that API, since I think future 
CPUs will allow that to be set too.  (If its not supported, then the 
call will fail if the scale is anything other than 1.)

     J

Re: [PATCH 0 of 5] PV on HVM Xen

[Stefano Stabellini]

On Tue, 16 Mar 2010, Jeremy Fitzhardinge wrote:
> On 03/16/2010 04:07 AM, Stefano Stabellini wrote:
> > OK, you are right: having an explicit "set/remove tsc offset" is
> > the best solution.
> >    
> 
> I wonder if we should include scale in that API, since I think future 
> CPUs will allow that to be set too.  (If its not supported, then the 
> call will fail if the scale is anything other than 1.)
> 

Actually I think Ian is right, we can do it without a new hypercall: if
we assume tsc_mode=2 then this simple patch makes the pv clocksource work
fine without any set_tsc_offset(v, 0) in xen:

---

diff -r 120b0e774c41 xen/arch/x86/time.c
--- a/xen/arch/x86/time.c	Mon Mar 15 15:05:15 2010 +0000
+++ b/xen/arch/x86/time.c	Tue Mar 16 17:26:08 2010 +0000
@@ -852,6 +852,10 @@
         _u.tsc_to_system_mul = t->tsc_scale.mul_frac;
         _u.tsc_shift         = (s8)t->tsc_scale.shift;
     }
+    if ( is_hvm_domain(d) )
+    {
+        _u.tsc_timestamp += v->arch.hvm_vcpu.cache_tsc_offset;
+    }
 
     /* Don't bother unless timestamp record has changed or we are forced. */
     _u.version = u->version; /* make versions match for memcmp test */

---

I tested this patch with my enhanced PV on HVM patch series.
If tsc_mode==1 everything gets more complicated, maybe Dan would know
what we need there to make it work.

Re: [PATCH 0 of 5] PV on HVM Xen

[Jeremy Fitzhardinge]

On 03/16/2010 10:32 AM, Stefano Stabellini wrote:
> Actually I think Ian is right, we can do it without a new hypercall: if
> we assume tsc_mode=2 then this simple patch makes the pv clocksource work
> fine without any set_tsc_offset(v, 0) in xen:
>    

Can we detect if this patch is present in Xen or not?  If not, we may 
need to go back to having a feature flag.

     J

Re: [PATCH 0 of 5] PV on HVM Xen

[Stefano Stabellini]

On Tue, 16 Mar 2010, Jeremy Fitzhardinge wrote:
> On 03/16/2010 10:32 AM, Stefano Stabellini wrote:
> > Actually I think Ian is right, we can do it without a new hypercall: if
> > we assume tsc_mode=2 then this simple patch makes the pv clocksource work
> > fine without any set_tsc_offset(v, 0) in xen:
> >    
> 
> Can we detect if this patch is present in Xen or not?  If not, we may 
> need to go back to having a feature flag.
> 

the following is the interesting bit of the pvclock interface:

static __always_inline
u64 pvclock_get_nsec_offset(const struct pvclock_vcpu_time_info *src)
{
	u64 delta = __native_read_tsc() - src->tsc_timestamp;
	return scale_delta(delta, src->tsc_to_system_mul, src->tsc_shift);
}


xen refreshes the values in pvclock_vcpu_time_info every EPOCH (1s),
therefore if the value returned by pvclock_get_nsec_offset is greater
than EPOCH than the patch is not present in xen.

This is a simple way of detecting if the offset is taken into account or
not and it works because the offset is the value returned by rdtsc in
the host when the VM was created and we can be sure it corresponds to
more than 1 sec.

Re: [PATCH 0 of 5] PV on HVM Xen

[Jeremy Fitzhardinge]

On 03/16/2010 11:06 AM, Stefano Stabellini wrote:
> the following is the interesting bit of the pvclock interface:
>
> static __always_inline
> u64 pvclock_get_nsec_offset(const struct pvclock_vcpu_time_info *src)
> {
> 	u64 delta = __native_read_tsc() - src->tsc_timestamp;
> 	return scale_delta(delta, src->tsc_to_system_mul, src->tsc_shift);
> }
>
>
> xen refreshes the values in pvclock_vcpu_time_info every EPOCH (1s),
> therefore if the value returned by pvclock_get_nsec_offset is greater
> than EPOCH than the patch is not present in xen.
>
> This is a simple way of detecting if the offset is taken into account or
> not and it works because the offset is the value returned by rdtsc in
> the host when the VM was created and we can be sure it corresponds to
> more than 1 sec.
>    

That seems pretty fragile.  I don't think EPOCH is part of the ABI, and 
I don't think we should be relying on it.

     J

Re: [PATCH 0 of 5] PV on HVM Xen

[Stefano Stabellini]

On Tue, 16 Mar 2010, Jeremy Fitzhardinge wrote:
> On 03/16/2010 11:06 AM, Stefano Stabellini wrote:
> > the following is the interesting bit of the pvclock interface:
> >
> > static __always_inline
> > u64 pvclock_get_nsec_offset(const struct pvclock_vcpu_time_info *src)
> > {
> > 	u64 delta = __native_read_tsc() - src->tsc_timestamp;
> > 	return scale_delta(delta, src->tsc_to_system_mul, src->tsc_shift);
> > }
> >
> >
> > xen refreshes the values in pvclock_vcpu_time_info every EPOCH (1s),
> > therefore if the value returned by pvclock_get_nsec_offset is greater
> > than EPOCH than the patch is not present in xen.
> >
> > This is a simple way of detecting if the offset is taken into account or
> > not and it works because the offset is the value returned by rdtsc in
> > the host when the VM was created and we can be sure it corresponds to
> > more than 1 sec.
> >    
> 
> That seems pretty fragile.  I don't think EPOCH is part of the ABI, and 
> I don't think we should be relying on it.
> 

EPOCH is certainly not part of the ABI :)
That said the difference between a correct delta and a wrong delta is so
big that we cannot really be mistaken.

In any case I wouldn't mind a feature flag just to stay on the safe
side, so I'll leave this decision to you and Keir (that this week is on
vacation).

Re: [PATCH 0 of 5] PV on HVM Xen

[Sheng Yang]

On Wednesday 17 March 2010 02:37:51 Stefano Stabellini wrote:
> On Tue, 16 Mar 2010, Jeremy Fitzhardinge wrote:
> > On 03/16/2010 11:06 AM, Stefano Stabellini wrote:
> > > the following is the interesting bit of the pvclock interface:
> > >
> > > static __always_inline
> > > u64 pvclock_get_nsec_offset(const struct pvclock_vcpu_time_info *src)
> > > {
> > > 	u64 delta = __native_read_tsc() - src->tsc_timestamp;
> > > 	return scale_delta(delta, src->tsc_to_system_mul, src->tsc_shift);
> > > }
> > >
> > >
> > > xen refreshes the values in pvclock_vcpu_time_info every EPOCH (1s),
> > > therefore if the value returned by pvclock_get_nsec_offset is greater
> > > than EPOCH than the patch is not present in xen.
> > >
> > > This is a simple way of detecting if the offset is taken into account
> > > or not and it works because the offset is the value returned by rdtsc
> > > in the host when the VM was created and we can be sure it corresponds
> > > to more than 1 sec.
> >
> > That seems pretty fragile.  I don't think EPOCH is part of the ABI, and
> > I don't think we should be relying on it.
> 
> EPOCH is certainly not part of the ABI :)
> That said the difference between a correct delta and a wrong delta is so
> big that we cannot really be mistaken.
> 
> In any case I wouldn't mind a feature flag just to stay on the safe
> side, so I'll leave this decision to you and Keir (that this week is on
> vacation).

I am quite happy with a feature flag. Depends on the return of hypercall to 
determine if the feature is there seems tricky to me...

Sure, I am fine with a set/remove tsc offset. And a side effort would be we 
need more code to do it explicitly for SMP guest's vcpu in the guest kernel 
code... But it's fine if they are elegant enough(I think setup_percpu_clockev 
should be fit for it).

-- 
regards
Yang, Sheng

Re: [PATCH 0 of 5] PV on HVM Xen

[Sheng Yang]

On Wednesday 17 March 2010 16:51:05 Sheng Yang wrote:
> On Wednesday 17 March 2010 02:37:51 Stefano Stabellini wrote:
> > On Tue, 16 Mar 2010, Jeremy Fitzhardinge wrote:
> > > On 03/16/2010 11:06 AM, Stefano Stabellini wrote:
> > > > the following is the interesting bit of the pvclock interface:
> > > >
> > > > static __always_inline
> > > > u64 pvclock_get_nsec_offset(const struct pvclock_vcpu_time_info *src)
> > > > {
> > > > 	u64 delta = __native_read_tsc() - src->tsc_timestamp;
> > > > 	return scale_delta(delta, src->tsc_to_system_mul, src->tsc_shift);
> > > > }
> > > >
> > > >
> > > > xen refreshes the values in pvclock_vcpu_time_info every EPOCH (1s),
> > > > therefore if the value returned by pvclock_get_nsec_offset is greater
> > > > than EPOCH than the patch is not present in xen.
> > > >
> > > > This is a simple way of detecting if the offset is taken into account
> > > > or not and it works because the offset is the value returned by rdtsc
> > > > in the host when the VM was created and we can be sure it corresponds
> > > > to more than 1 sec.
> > >
> > > That seems pretty fragile.  I don't think EPOCH is part of the ABI, and
> > > I don't think we should be relying on it.
> >
> > EPOCH is certainly not part of the ABI :)
> > That said the difference between a correct delta and a wrong delta is so
> > big that we cannot really be mistaken.
> >
> > In any case I wouldn't mind a feature flag just to stay on the safe
> > side, so I'll leave this decision to you and Keir (that this week is on
> > vacation).
> 
> I am quite happy with a feature flag. Depends on the return of hypercall to
> determine if the feature is there seems tricky to me...
> 
> Sure, I am fine with a set/remove tsc offset.

Seem not get enough update...

OK, a new flag, adjustment in Xen. Right?

-- 
regards
Yang, Sheng

> And a side effort would be we
> need more code to do it explicitly for SMP guest's vcpu in the guest kernel
> code... But it's fine if they are elegant enough(I think
>  setup_percpu_clockev should be fit for it).
> 

Re: [PATCH 0 of 5] PV on HVM Xen

[Stefano Stabellini]

On Wed, 17 Mar 2010, Sheng Yang wrote:
> Seem not get enough update...
> 
> OK, a new flag, adjustment in Xen. Right?
> 


Yes, a new flag to signal the presence of a reliable clocksource on HVM;
adjustments in Xen to make it work (keep in mind that my patch fix the
problem only when tsc_mode==2 and we need to support tsc_mode==1 too).

On the other hand we agreed that we don't need XEN_HVM_PV_EVTCHN_ENABLED
and CONFIG_XEN_HVM_PV anymore.
We probably don't need XEN_HVM_PV too for the moment, we might introduce
it in the future when we actually add code that doesn't work on 32 bit.

Finally I would still like the call to xen_guest_init to be moved
afterwards: if we move it after kvm_guest_init we can be pretty sure
that upstream is going to accept it. Besides ACPI is currently working
with your patch series applied, when and if we break ACPI we'll worry
about it.

Jeremy, Ian, does this seem reasonable to you? The last point in
particular?  If you are sure that upstream will accept a hook in setup.c
anyway I am ready to drop this.

Re: [PATCH 0 of 5] PV on HVM Xen

[Ian Campbell]

On Wed, 2010-03-17 at 15:17 +0000, Stefano Stabellini wrote:
> On Wed, 17 Mar 2010, Sheng Yang wrote:
> > Seem not get enough update...
> > 
> > OK, a new flag, adjustment in Xen. Right?
> > 
> 
> 
> Yes, a new flag to signal the presence of a reliable clocksource on HVM;
> adjustments in Xen to make it work (keep in mind that my patch fix the
> problem only when tsc_mode==2 and we need to support tsc_mode==1 too).
> 
> On the other hand we agreed that we don't need XEN_HVM_PV_EVTCHN_ENABLED
> and CONFIG_XEN_HVM_PV anymore.
> We probably don't need XEN_HVM_PV too for the moment, we might introduce
> it in the future when we actually add code that doesn't work on 32 bit.
> 
> Finally I would still like the call to xen_guest_init to be moved
> afterwards: if we move it after kvm_guest_init we can be pretty sure
> that upstream is going to accept it. Besides ACPI is currently working
> with your patch series applied, when and if we break ACPI we'll worry
> about it.
> 
> Jeremy, Ian, does this seem reasonable to you? The last point in
> particular?  If you are sure that upstream will accept a hook in setup.c
> anyway I am ready to drop this.

I think that if we can we should avoid disabling ACPI, and if we don't
need to do that then I'm not sure we need the Xen initialisation point
to be all that early. I would think that the location of the KVM init
point is likely to be a good choice for Xen as well, in the absence of
other considerations there's a pretty strong argument for keeping these
virtualisation entry points in the same place.

It's not like we can't move the hook earlier in the future if that turns
our to be unavoidably necessary.

Ian.

Re: [PATCH 0 of 5] PV on HVM Xen

[Sheng Yang]

On Thursday 18 March 2010 02:20:28 Ian Campbell wrote:
> On Wed, 2010-03-17 at 15:17 +0000, Stefano Stabellini wrote:
> > On Wed, 17 Mar 2010, Sheng Yang wrote:
> > > Seem not get enough update...
> > >
> > > OK, a new flag, adjustment in Xen. Right?
> >
> > Yes, a new flag to signal the presence of a reliable clocksource on HVM;
> > adjustments in Xen to make it work (keep in mind that my patch fix the
> > problem only when tsc_mode==2 and we need to support tsc_mode==1 too).
> >
> > On the other hand we agreed that we don't need XEN_HVM_PV_EVTCHN_ENABLED
> > and CONFIG_XEN_HVM_PV anymore.
> > We probably don't need XEN_HVM_PV too for the moment, we might introduce
> > it in the future when we actually add code that doesn't work on 32 bit.
> >
> > Finally I would still like the call to xen_guest_init to be moved
> > afterwards: if we move it after kvm_guest_init we can be pretty sure
> > that upstream is going to accept it. Besides ACPI is currently working
> > with your patch series applied, when and if we break ACPI we'll worry
> > about it.
> >
> > Jeremy, Ian, does this seem reasonable to you? The last point in
> > particular?  If you are sure that upstream will accept a hook in setup.c
> > anyway I am ready to drop this.
> 
> I think that if we can we should avoid disabling ACPI, and if we don't
> need to do that then I'm not sure we need the Xen initialisation point
> to be all that early. 

Yes, the issue is ACPI. 

> I would think that the location of the KVM init
> point is likely to be a good choice for Xen as well, in the absence of
> other considerations there's a pretty strong argument for keeping these
> virtualisation entry points in the same place.

But I don't think this argument is strong enough...

Look back to setup_arch(), you can see this:

Line		Function
696		vmi_init()
794		vmi_activate()
966		kvmclock_init()
1008		kvm_guest_init()

The code is already sparse... Each function have different requirement(before 
xxx, or after xxx), so it is quite understandable...

-- 
regards
Yang, Sheng

> 
> It's not like we can't move the hook earlier in the future if that turns
> our to be unavoidably necessary.
> 
> Ian.
> 

Re: [PATCH 0 of 5] PV on HVM Xen

[Sheng Yang]

On Wednesday 17 March 2010 23:17:08 Stefano Stabellini wrote:
> On Wed, 17 Mar 2010, Sheng Yang wrote:
> > Seem not get enough update...
> >
> > OK, a new flag, adjustment in Xen. Right?
> 
> Yes, a new flag to signal the presence of a reliable clocksource on HVM;
> adjustments in Xen to make it work (keep in mind that my patch fix the
> problem only when tsc_mode==2 and we need to support tsc_mode==1 too).
> 
> On the other hand we agreed that we don't need XEN_HVM_PV_EVTCHN_ENABLED
> and CONFIG_XEN_HVM_PV anymore.

No XEN_HVM_PV_EVTCHN_ENABLED? So you have to enable HVM with evtchn support? I 
am not quite understand...

-- 
regards
Yang, Sheng


> We probably don't need XEN_HVM_PV too for the moment, we might introduce
> it in the future when we actually add code that doesn't work on 32 bit.
> 
> Finally I would still like the call to xen_guest_init to be moved
> afterwards: if we move it after kvm_guest_init we can be pretty sure
> that upstream is going to accept it. Besides ACPI is currently working
> with your patch series applied, when and if we break ACPI we'll worry
> about it.
> 
> Jeremy, Ian, does this seem reasonable to you? The last point in
> particular?  If you are sure that upstream will accept a hook in setup.c
> anyway I am ready to drop this.

Re: [PATCH 0 of 5] PV on HVM Xen

[Stefano Stabellini]

On Thu, 18 Mar 2010, Sheng Yang wrote:
> On Wednesday 17 March 2010 23:17:08 Stefano Stabellini wrote:
> > On Wed, 17 Mar 2010, Sheng Yang wrote:
> > > Seem not get enough update...
> > >
> > > OK, a new flag, adjustment in Xen. Right?
> > 
> > Yes, a new flag to signal the presence of a reliable clocksource on HVM;
> > adjustments in Xen to make it work (keep in mind that my patch fix the
> > problem only when tsc_mode==2 and we need to support tsc_mode==1 too).
> > 
> > On the other hand we agreed that we don't need XEN_HVM_PV_EVTCHN_ENABLED
> > and CONFIG_XEN_HVM_PV anymore.
> 
> No XEN_HVM_PV_EVTCHN_ENABLED? So you have to enable HVM with evtchn support? I 
> am not quite understand...
> 

You are not using it at the moment so there is no point in defining it.

Also XEN_HVM_PV_EVTCHN_ENABLED is not a good name for it because we'll
be able to receive evtchns anyway using xen platform device interrupts.
Maybe XEN_HVM_IPI_CALLBACK_ENABLE?

Re: [PATCH 0 of 5] PV on HVM Xen

[Jeremy Fitzhardinge]

On 03/18/2010 07:22 AM, Stefano Stabellini wrote:
> Also XEN_HVM_PV_EVTCHN_ENABLED is not a good name for it because we'll
> be able to receive evtchns anyway using xen platform device interrupts.
> Maybe XEN_HVM_IPI_CALLBACK_ENABLE?
>    

I'm OK with the original name.  The point is not that we're getting 
events from evtchns, but that the mechanism allows us to receive them 
without having to touch the APICs and suffer the vmexits.  To the rest 
of the kernel, events delivered via the platform device are really just 
ordinary interrupts from an ordinary device which happens to have a 
particularly elaborate "interrupt source" register.

     J

Re: [PATCH 0 of 5] PV on HVM Xen

[Jeremy Fitzhardinge]

On 03/17/2010 08:17 AM, Stefano Stabellini wrote:
> Finally I would still like the call to xen_guest_init to be moved
> afterwards: if we move it after kvm_guest_init we can be pretty sure
> that upstream is going to accept it. Besides ACPI is currently working
> with your patch series applied, when and if we break ACPI we'll worry
> about it.
>    

Why/how does ACPI get broken?  I think that's something we should 
definitely avoid doing.

     J

Re: [Xen-devel] [PATCH 0 of 5] PV on HVM Xen

[Jeremy Fitzhardinge]

On 03/12/2010 02:42 AM, Stefano Stabellini wrote:
> On Fri, 12 Mar 2010, Sheng Yang wrote:
>    
>> On Wednesday 10 March 2010 23:46:54 Stefano Stabellini wrote:
>>      
>>> Hi all,
>>>        
>> Stefano,
>>
>> And next time when you send out the patch, please be more respect to my work.
>>
>> You dropped all the original author(me) of patchset, and only add a sign-off
>> for me. If you don't aware the difference, here is a snippet of
>> linux/Documentation/SummittingPatches
>>
>>      
> I am truly sorry and apologise for it, I would never want to give you
> the impression of being disrespectful of you and your work.
> If you pay attention I manually wrote in the comments of all the past
> versions of the patches that you were the original author, this time I
> just forgot.
> I guess it is really the time I start using git-send-email :)
>
> Your work has been really important for my series and you deserve the
> credit for it independently from which patch series gets applied.
>    

Best practise for this kind of thing is to use Sheng's patch verbatim, 
then apply a separate delta on top to make the changes you want.  That way:

    * credit can be property attributed
    * it helps with maintaining two parallel-but-converging git
      branches, because all the common stuff is genuinely common, and
      the variations can work from there, and
    * if Sheng updates his patches (review, bugfix, etc), then we can
      easily see how those changes affect your changes.

For example, I just rebased your previous patch posting so that it is 
rooted on Sheng's two base patches, as they're completely common.

The latest of both patches is in xen/pvhvm-sheng and -stefano.

     J