Re: [Xen-devel] ARM fixes for my improved privcmd patch.

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Tue, Dec 18, 2012 at 01:51:49PM +0000, Mats Petersson wrote:
> On 18/12/12 11:40, Ian Campbell wrote:
> >On Tue, 2012-12-18 at 11:28 +0000, Mats Petersson wrote:
> >>On 18/12/12 11:17, Ian Campbell wrote:
> >>>On Mon, 2012-12-17 at 17:38 +0000, Mats Petersson wrote:
> >>>>On 17/12/12 16:57, Ian Campbell wrote:
> >>>>>On Fri, 2012-12-14 at 17:00 +0000, Mats Petersson wrote:
> >>>>>>Ian, Konrad:
> >>>>>>I took Konrad's latest version [I think] and applied my patch (which
> >>>>>>needed some adjustments as there are other "post 3.7" changes to same
> >>>>>>source code - including losing the xen_flush_tlb_all() ??)
> >>>>>>
> >>>>>>Attached are the patches:
> >>>>>>arm-enlighten.patch, which updates the ARM code.
> >>>>>>improve-pricmd.patch, which updates the privcmd code.
> >>>>>>
> >>>>>>Ian, can you have a look at the ARM code - which I quickly hacked
> >>>>>>together, I haven't compiled it, and I certainly haven't tested it,
> >>>>>There are a lot of build errors as you might expect (patch below, a few
> >>>>>warnings remain). You can find a cross compiler at
> >>>>>http://www.kernel.org/pub/tools/crosstool/files/bin/x86_64/4.6.3/
> >>>>>
> >>>>>or you can use
> >>>>>drall:/home/ianc/devel/cross/x86_64-gcc-4.6.0-nolibc_arm-unknown-linux-gnueabi.tar.bz2
> >>>>>
> >>>>>which is an older version from the same place.
> >>>>>
> >>>>>Anyway, the patch...
> >>>>>>and
> >>>>>>it needs further changes to make my changes actually make it more
> >>>>>>efficient.
> >>>>>Right, the benefit on PVH or ARM would be in batching the
> >>>>>XENMEM_add_to_physmap_range calls. The batching of the
> >>>>>apply_to_page_range which this patch adds isn't useful because there is
> >>>>>no HYPERVISOR_mmu_update call to batch in this case. So basically this
> >>>>>patch as it stands does a lot of needless work for no gain I'm afraid.
> >>>>So, basically, what is an improvement on x86 isn't anything useful on
> >>>>ARM, and you'd prefer to loop around in privcmd.c calling into
> >>>>xen_remap_domain_mfn_range() a lot of times?
> >>>Not at all. ARM (and PVH) still benefits from the interface change but
> >>>the implementation of the benefit is totally different.
> >>>
> >>>For normal x86 PV you want to batch the HYPERVISOR_mmu_update.
> >>>
> >>>For both x86 PVH and ARM this hypercall  doesn't exist but instead there
> >>>is a call to HYPERVISOR_memory_op XENMEM_add_to_physmap_range which is
> >>>something which would benefit from batching.
> >>So, you want me to fix that up?
> >If you want to sure, yes please.
> >
> >But feel free to just make the existing code work with the interface,
> >without adding any batching. That should be a much smaller change than
> >what you proposed.
> >
> >(aside; I do wonder how much of this x86/arm code could be made generic)
> I think, once it goes to PVH everywhere, quite a bit (as I believe
> the hypercalls should be the same by then, right?)
> 
> In the PVOPS kernel, it's probably a bit more job. I'm sure it can
> be done, but with a bit more work.
> 
> I think I'll do the minimal patch first, then, if I find some spare
> time, work on the "batching" variant.

OK. The batching is IMHO just using the multicall variant.

> >
> >>To make xentrace not work until it is fixed wouldn't be a terrible
> >>thing, would it?
> >On ARM I think it is fine (I doubt this is the only thing stopping
> >xentrace from working). I suspect people would be less impressed with
> >breaking xentrace on x86. For PVH it probably is a requirement for it to
> >keep working, I'm not sure though.
> Ok, ENOSYS it is for remap_range() then.
> >
> >>  Then we can remove that old gunk from x86 as well
> >>(eventually).
> >>Thanks. I was starting to wonder if I'd been teleported back to the time
> >>when I struggled with pointers...
> >>Maybe it needs a better comment.
> >The other thing I had missed was that this was a pure increment and not
> >taking the value at the same time, which also confused me.
> >
> >Splitting the increment out from the dereference usually makes these
> >things clearer, I was obviously just being a bit hard of thinking
> >yesterday!
> No worries. I will see about making a more readable comment (and for
> ARM, I can remove the whole if/else and just do the one increment
> (based on above discussion), so should make the code better.

You can use the v3.8 tree as your base - it has the required PVH and ARM
patches. There is one bug (where dom0 crashes) - and I just sent
a git pull for that in your Linus's tree:

 git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git 
stable/for-linus-3.8

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel