Re: [Xen-devel] [PATCH 1/6] Flask: Support for ARM xentrace
Konrad, > Modified to provide support for xentrace on the ARM platform. Added > flask credential to allow dom0 dom_xen mapping and write access for > trace buffers. >> >> So .. what does that mean? >> >> Is that something xentrace requests? Why is this ARM specific? >> Looking at xsm_sysctl and how the trace is setup it checks for >> XEN__TBUFCONTROL? I need to bring in Paul for this conversation. The Flask modifications were originally made by him so I'm not sure on their exact purpose or derivation. Paul: Can you provide any insight as to the changes to the Flask code? Thanks, Ben ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 3/6] xentrace: P2M lookup suport for ARM platform
Julien and George, Thank you for the comments. I had one question I wanted to ask. >A DOMID_XEN page could be read only too. For instance, the meta-data >of the trace buffer is read-only (see t_info), we don't want a domain >to be able to overwrite them. >However, all the foreign page are mapped read-write. You will need to >rework the code to map a foreign domain (see >XENMAPSPACE_gmfn_foreign) to allow read-only foreign page (maybe by >adding a new p2m_type_t?). I understand what you are saying in general, but I'm not familiar enough with the Xen memory mapping system to know how to actually implement this. The p2m_type_t p2m_ram_ro exists, which I could assign to read-only pages, but I'm unsure as to how to detect whether a request is to a read only mapping or a read-write. The normal (non DOMID_XEN) p2m_lookup function normally does this by reading the root- level page tables and somehow extracting the mapping type from the lpae_t structure. Given that we are not looking up the page tables for non-translated addresses, I'm not sure where/how to find the correct mapping type. Can I still lookup the page table entries for the MFN address and extract the p2m_type_t the same way? Thank you for any insight or assistance, Ben ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 6/6] xentrace: ARM platform timestamp support
Stefano, Thank you for the comments. In response: > Changing cycles_t to uint64_t sounds good, but why did you move > boot_count here from below? I had to move it up so it would be defined for use in the updated get_cycles() call: return READ_SYSREG64(CNTPCT_EL0) - boot_count; Should get_cyclces not use boot_count? I included it as the normal system time call uses it so I assumed this should be consistent. Thanks, Ben Sanda ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V2 0/9] xentrace/xenalyze support on ARM
Julien and Wei, >> You patches all have the same subject line. Please make them more >> specific. See my reply to #1 for example. > > +1 > > Also, you should at least check that Xen still builds after applying > each patch. Ideally, you also need to be careful to not break any > feature currently supported. It's useful when someone needs to > bisect the tree. > > For instance, you use the function get_pg_owner for ARM in patch #2 > but introduce the function in patch #4. This will break ARM build. > So the patch #2 should be moved after #4. > > Furthermore, you remove the functions get_pg_owner and put_pg_owner > for x86 in patch #3 and then re-introduced them in patch #4. > Therefore, the x86 will be broken after #3. In this case, it's better > to have a patch that move the 2 functions from x86 to common. Thank you for the comments. I apologize for the errors in the patch format. This is my first time submitting a patch to Xen and I was unaware that the patch set order mattered or that I had to account for a piecewise application of the patch set. I will attempt to resubmit with this corrected and the patch names updated. So then it is permissible to have multiple file changes in one patch/commit? E.g. a patch that removes from one file and adds to another in the same commit. I initially thought each patch/ commit was only supposed to modify one file and that's why I did it that way Thank you, Ben Sanda ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 0/5] xentrace/xenalyze Support on ARM
Jan,
> A couple of formal things: This is v3, and I only now notice
> indeed I should have looked at some of the patches. Yet which of them
> isn't clear - I'm being Cc-ed on all of them, instead of just the ones
> that submission guidelines say I should be. And then all patch
> subjects start with xenalyze: or xentrace:, suggesting this series
> isn't touching code other than those two. Generalization of
> {get,put}_pg_owner(), otoh, while apparently a prereq for the
> generalization of one or both of the two, should use a more indicative
> component prefix (or maybe even none at all).
Thank you for the comments, I will update the names of the patches so
they are more obvious as to what they touch. As far as the CC'ing
though, I'm not sure how to do this. When I create the patch and send
it out with git send-email I send one email for the entire patch set
with one CC list. I don't know of a method to send a patch set but
have each individual patch go to a different CC list.
Thanks,
Ben
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 0/5] xentrace/xenalyze Support on ARM
Andrew, > Word your commit message like so: > > Signed-off-by: $ME > --- > CC: $SOMEONE > CC: $SOMEONE_ELSE > > and `git send-email` will DTRT. Ah, excellent. Then git send-email will auto add. Thanks, I'll make the change for v4. Ben ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 2/5] xentrace: Memory/Page Mapping support for DOMID_XEN on ARM
Julien, George, Andrew, and Stefano, > Thank you for the explanation. > > The ARM implementation of share_xen_page_with_guest is nearly the same > as the x86 one. However, the type is never used so far for the P2M > code. > > So far, all ARM domains have been auto-translated. DOMID_XEN is the > first non auto-translated domain. > > We could make DOMID_XEN an auto-translated domain by introducing page > table for dummy domain. This would make the code cleaner but use more > memory (allocation of 3 level of page tables). > > Stefano, do you have any opinions on this? How would you like to me proceed with this issue with regard to the patch set? It sounds like this is a more far reaching architecture design question for ARM. I have made the other changes to this version of the code requested, should I submit v4? Or wait for a further resolution to this discussion? Thanks, Ben Sanda ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 1/5] xentrace: Common Support for get_pg_owner/put_pg_owner on ARM and x86
Jan,
>> +void put_pg_owner(struct domain *pg_owner) {
>> +rcu_unlock_domain(pg_owner);
>> +}
> I cannot see why this then can't just become an inline function.
I investigated this but making put_pg_owner() static inline creates a
circular dependency on rcu_unlock_domain(), which is also a static
inline function. The two functions are in different header files and
this creates a dependency on the one header being included by the
other, which, depending on how C files include them, creates an
implicit definition error by the compiler. For now I will leave the
function as is.
Thanks,
Ben Sanda
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] Xentrace on Xilinx ARM
Hello,
My name is Ben Sanda, I'm a kernel/firmware developer with DornerWorks
engineering. Our team is working on support for Xen on the new Xilinx
Ultrascale+ MPSoC platforms (ARM A53 core) and I've specifically been tasked
with characterizing performance, particularly that of the schedulers. I wanted
to make use of the xentrace tool to help give us some timing and performance
benchmarks, but searching over the Xen mailing lists it appears xentrace has not
yet been ported to ARM. When you run it crashes complaining about allocating
memory buffers. While we could just write some custom quick program to
collect the data we need, I would rather help get xentrace working on ARM
so is generally available to everyone and usable for any benchmarking
moving forward.
In searching for existing topics on this my main reference thread for this has
been the "[Xen-devel] xentrace, arm, hvm" email chain started by Pavlo Suikov
here: http://xen.markmail.org/thread/zochggqxcifs5cdi
I have been trying to follow that email chain, which made some suggestions as to
how xentrace could be ported to ARM and where things are going wrong, but it
never came to any concrete conclusions. I have gathered from the thread that
there are issues with the memory allocation for the xentrace buffers due to MFN
and PFN mapping differences on the ARM vs x86 when attempting to map
from the XEN HEAP. I followed the suggestions posed by the thread as follows
(performed against the Xilinx/master Git version of the Xen source here:
https://github.com/Xilinx/xen):
First, in mm.c, I modified the xenmem_add_to_physmap_one() and its call to
rcu_lock_domain_by_any_id() call to provide a special case for the DOM_XEN
domain request. For this I created a new function, get_pg_owner(), which does
the same domain checks as get_pg_owner() on the x86 source performs. This allows
the correct domid_t to be returned.
--- xen-src/xen/arch/arm/mm.c 2016-03-04 10:44:31.364572302 -0800
+++ xen-src_mod/xen/arch/arm/mm.c 2016-02-24 09:41:43.0 -0800
@@ -41,6 +41,7 @@
#include
struct domain *dom_xen, *dom_io, *dom_cow;
+static struct domain *get_pg_owner(domid_t domid);
/* Static start-of-day pagetables that we use before the allocators
* are up. These are used by all CPUs during bringup before switching
@@ -1099,7 +1100,8 @@
{
struct domain *od;
p2m_type_t p2mt;
-od = rcu_lock_domain_by_any_id(foreign_domid);
+od = get_pg_owner(foreign_domid);
+
if ( od == NULL )
return -ESRCH;
@@ -1132,7 +1134,15 @@
return -EINVAL;
}
-mfn = page_to_mfn(page);
+if(od->domain_id != DOMID_XEN)
+{
+mfn = page_to_mfn(page);
+}
+else
+{
+mfn = idx;
+}
+
t = p2m_map_foreign;
rcu_unlock_domain(od);
@@ -1312,6 +1321,42 @@
unmap_domain_page(p);
}
+static struct domain *get_pg_owner(domid_t domid)
+{
+struct domain *pg_owner = NULL, *curr = current->domain;
+
+if ( likely(domid == DOMID_SELF) )
+{
+pg_owner = rcu_lock_current_domain();
+goto out;
+}
+
+if ( unlikely(domid == curr->domain_id) )
+{
+goto out;
+}
+
+switch ( domid )
+{
+case DOMID_IO:
+pg_owner = rcu_lock_domain(dom_io);
+break;
+case DOMID_XEN:
+/*printk("DOM_XEN Selected\n");*/
+pg_owner = rcu_lock_domain(dom_xen);
+break;
+default:
+if ( (pg_owner = rcu_lock_domain_by_id(domid)) == NULL )
+{
+break;
+}
+break;
+}
+
+ out:
+return pg_owner;
+}
Second I modified p2m_lookup() in p2m.c to manage the fact that xentrace
provides a MFN, not a PFN to the domain lookup calls. It now checks for DOM_XEN,
and if found, returns the MFN directly instead of trying to translate it from a
PFN. It also sets the page type to p2m_raw_rw. (Which I guessed was the correct
type for read/write to the XEN Heap? I'm not sure if that's correct.)
@@ -228,10 +228,19 @@
paddr_t ret;
struct p2m_domain *p2m = &d->arch.p2m;
-spin_lock(&p2m->lock);
-ret = __p2m_lookup(d, paddr, t);
-spin_unlock(&p2m->lock);
+if(d->domain_id != DOMID_XEN)
+{
+spin_lock(&p2m->lock);
+ret = __p2m_lookup(d, paddr, t);
+spin_unlock(&p2m->lock);
+}
+else
+{
+*t = p2m_ram_rw;
+ret = paddr;
+}
+
return ret;
}
The result is that now when I call xentrace() no errors are reported, however; I
also don't observe any trace logs actually being collected. I can invoke
xentrace (as xentrace -D -S 256 -t 10 -e all out.bin), and I see xentrace start,
and out.bin created, but it's always empty.
[root@xilinx-dom0 ~]# xentrace -D -S 256 -T 10 -e all out.bin
change evtmask to 0x000
(XEN) xentrace: requesting 2 t_info pages for 256 trace pa
Re: [Xen-devel] Xentrace on Xilinx ARM
Dario, Thank you very much for the help. I apologize for the HTML output on the first email, I thought I had outlook set to send it in plain text. My mistake. > Well, in this other thread, Paul (Cc-ed) says he basically has tracing > working on ARM: > http://lists.xenproject.org/archives/html/xen-devel/2016-02/msg03373.html I hadn't found that thread by Paul thank you for pointing it out. I would be eager to see what additional changes he had to make to get it actually working. It sounds like we headed down the same path, but there's more needed that I'm unaware of. Paul, I would be eager to see what changes you had to make to get xentrace working on ARM and compare that against what I've tried. If we could push up a formal patch that would be excellent. Thanks, Ben -Original Message- From: Dario Faggioli [mailto:dario.faggi...@citrix.com] Sent: 05 March, 2016 10:43 To: Ben Sanda ; xen-devel@lists.xen.org Cc: Paul Sujkov Subject: Re: [Xen-devel] Xentrace on Xilinx ARM On Fri, 2016-03-04 at 20:53 +, Ben Sanda wrote: > Hello, > Hello, first of all, please, use plain text instead of HTML for emails to this list. > My name is Ben Sanda, I’m a kernel/firmware developer with DornerWorks > engineering. Our team is working on support for Xen on the new Xilinx > Ultrascale+ MPSoC platforms (ARM A53 core) and I’ve specifically been > tasked > with characterizing performance, particularly that of the schedulers. > I wanted > to make use of the xentrace tool to help give us some timing and > performance benchmarks, but searching over the Xen mailing lists it > appears xentrace has not yet been ported to ARM. > No, tracing support for ARM is not present upstream > In searching for existing topics on this my main reference thread for > this has been the “[Xen-devel] xentrace, arm, hvm” email chain started > by Pavlo Suikov > here: http://xen.markmail.org/thread/zochggqxcifs5cdi > Well, in this other thread, Paul (Cc-ed) says he basically has tracing working on ARM: http://lists.xenproject.org/archives/html/xen-devel/2016-02/msg03373.html Any chance you maybe to can cooperate to get such support upstream? That would be reallyy cool. :-) Regards, Dario -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Xentrace on Xilinx ARM
Paul, Thank you very much for the reply. I’ll check out the fork for xenalyze. I believe I have xentrace working now, mostly, but if possible I’d like to see what you did on your build to get it working to compare. I would like to push a patch into the mainline that agrees with both of our efforts. Thanks, Ben ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Xentrace on Xilinx ARM
Dario, I gleamed enough information from Paul's posts that I now have xentrace outputting data (I don't know if it's correct data or gibberish yet though). To discover this I tried to find the xenalyze tool but have not been able to figure out how to get it built. According to the old documentation at: https://blog.xenproject.org/2012/09/27/tracing-with-xentrace-and-xenalyze/ it was in a mercurial repo here: http://xenbits.xensource.com/ext/xenalyze.hg but that repo is no longer functional it seems. I searched through the mailing lists and it looks like xenalyze was pulled into the mainline and now resides in tools/xentrace. I can't determine how to get it to build though. I've tried calling make in the directory but that fails. I'm using petalinux which has a build xen tools make object, which I have also tried, and it generates an object file for xenalyze.c, but no executable. Could you provide any guidance as to how to actually get xenalyze built? I'm assuming it's still an offline tool? Or is it now built into the Xen image? Thank you, Ben Sanda ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Xentrace on Xilinx ARM
All, To update to the current situation. I have been able to get xentrace() and xenalyze working completely (at least as far as I can tell) on ARM. For xentrace there were changes to the memory allocation routines to allow mapping of the Xen Heap by dom0, correcting the MFN->PFN translations, adding the trace buffer initialization to setup.c (init_trace_bufs), and correcting the get_cycles() call to provide the system TSC. For the get_cycles() call I gathered that was supposed to return the raw tick count, not a translated ticks->real time timestamp. I then had to call xenalyze with the core frequency defined so the timestamps made sence. Paul: Was there anything else you did I missed? >It's not part of any Xen image. It's a command line tool to be used, usually >but not necessarily, in dom0, build and installed together with the other >tools... At least in my case, for x86 builds and installs. For xenalyze I had to modify the makefile to build xenalyze on the ARM platform (it was specifically removed from the ARM build). Once that was corrected I could find and call it from dom0. It built only locally to Xen though (could only run from dom0), I could not use it from the native Linux development environment (I don't know if you're supposed to be able to? Or since I'm running ARM it built for ARM not x86 and thus could not be used natively). I plan to push they changes in as a patch to the mainline if that seems reasonable to everyone. Thanks, Ben Sanda ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Xentrace on Xilinx ARM
George, >FWIW, on my "to-do" list for xenalyze for years has been to have the xentrace >process query something (either Xen or Linux) to find the hz rate, and then >write that at the beginning of the xentrace file, so that xenalyze could just >pick that up and use it. Since you're doing some work on xentrace / xenalyze >anyway, you might think about adding that to your "to-do" list -- it doesn't >seem conceptually like it would be that hard. Since I'm in the code base anyway right now I'll give it a look. I was wondering about that for the xenalyze hertz parameter. I had assumed initially xentrace embedded the value somehow already. Is there any documentation as to the binary format xentrace outputs? Or is it best just to look at the code and figure it out? I guess the main thing would be providing a common interface for both ARM and x86 but the common trace.c has a get frequency function that's implemented by both so we should be able to query it and shove in the data. >The other thing that might be useful is information about the architecture >you're running on -- right now it assumes intel, and will crash tracing a file >generated on an AMD box unless you specify --svm-mode. Adding a trace record >that indicates "Intel / AMD / ARM" would also make things a lot easier. I didn't run into any issue with xenalyze analyzing the ARM generated trace file (I didn't give it any special flag). What does --svm-mode modify? The endedness? I can look quick into that as well. Thanks, Ben ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Xentrace on Xilinx ARM
Paul, >see the patch attached. It's a bit dirty still (that's why it wasn't >upstreamed >long time ago), but you can obviously look through it and gain some >information. Thanks, I’ll take a look and compare it against mine to make sure we capture everything. Ben ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Xentrace on Xilinx ARM
George, Thank you for all the information. I'll take a dive in and see what I can see. I'd at least like to get the CPU core frequency detection in as that will make things a lot easier for us as we port to multiple platforms with different cores. Then we don't have to keep looking up core speeds. Ben ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/6] xentrace/xenalyze support on ARM
Julien, >How did you invoke get_maintainers.pl? > >I tried with the patch #4 and got the following ouput: > >42sh$ scripts/get_maintainers.pl < >xentrace-ARM-platform-DOMID_XEN-mapping-support.patch > >Stefano Stabellini Julien Grall > xen-devel@lists.xen.org Since I had a multifile patch I invoked it on the folder generated by git send-email --format-patch ./scripts/get_maintainer.pl -f new_patch Reading the help usage it indicated you could run it on folders or directories. It would seem you have to run it on a per file basic though? I will be sure to correct this moving forward. Thanks, Ben -Original Message- From: Julien Grall [mailto:julien.gr...@arm.com] Sent: 17 March, 2016 13:01 To: Ben Sanda ; xen-de...@lists.xenproject.org Subject: Re: [Xen-devel] [PATCH 0/6] xentrace/xenalyze support on ARM Hello Benjamin, On 17/03/16 16:50, Ben Sanda wrote: >> Can you please CC the relevant maintainers in the next version of this >> series? >> You can use scripts/get_maintainers.pl for this purpose. > > I did. The list output to me was: > Ian Jackson Jan Beulich > Keir Fraser Tim Deegan > How did you invoke get_maintainers.pl? I tried with the patch #4 and got the following ouput: 42sh$ scripts/get_maintainers.pl < xentrace-ARM-platform-DOMID_XEN-mapping-support.patch Stefano Stabellini Julien Grall xen-devel@lists.xen.org Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/6] xentrace/xenalyze support on ARM
Julien, > Can you please CC the relevant maintainers in the next version of this series? > You can use scripts/get_maintainers.pl for this purpose. I did. The list output to me was: Ian Jackson Jan Beulich Keir Fraser Tim Deegan I also added a few more that I had already been in contact with regarding this effort but no other names were output. I can be sure to include you in future updates. Ben ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
