| Message ID | 1560318154-19095-1-git-send-email-chenbaodong@mxnavi.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: io: add function swap_mmio_handler() | expand |
Hi, On 6/12/19 6:42 AM, Baodong Chen wrote: > Swap function can be used when calling sort(). > or else, the default swap function generic_swap() is used, > which is a little inefficient. I am not entirely convince this will be more efficient. mmio_handler does not fit in 64 bit, so the compiler may decide to do either multiple load or replace with a memcpy. So at best this feels some micro-optimization. But then, this is only call a limited number of time at each domain build. Is it really worth it? On a side note, I have noticed you are sending a lot of optimization/clean-up patch. What is your end goal here? If it is to improve the performance, then there are much bigger fish to fry within Xen code base. I am happy to point some of them based on where you are looking to improve. Cheers,
On 6/12/19 17:08, Julien Grall wrote: > Hi, > > On 6/12/19 6:42 AM, Baodong Chen wrote: >> Swap function can be used when calling sort(). >> or else, the default swap function generic_swap() is used, >> which is a little inefficient. > > I am not entirely convince this will be more efficient. mmio_handler > does not fit in 64 bit, so the compiler may decide to do either > multiple load or replace with a memcpy. Hello Julien, I have checked the disassemble result, and IIUC generic_swap has a loop so it should be a little inefficient. I'm not expert about hardware, please correct me if i'm wrong. 000000000022ee88 <generic_swap>: 22ee88: d2800003 mov x3, #0x0 // #0 22ee8c: d503201f nop 22ee90: 38636825 ldrb w5, [x1, x3] 22ee94: 38636804 ldrb w4, [x0, x3] 22ee98: 38236805 strb w5, [x0, x3] 22ee9c: 38236824 strb w4, [x1, x3] 22eea0: 91000463 add x3, x3, #0x1 22eea4: 4b030044 sub w4, w2, w3 22eea8: 7100009f cmp w4, #0x0 22eeac: 54ffff2c b.gt 22ee90 <generic_swap+0x8> 22eeb0: d65f03c0 ret 22eeb4: d503201f nop 0000000000242db8 <swap_mmio_handler>: 242db8: a9400c22 ldp x2, x3, [x1] 242dbc: d10083ff sub sp, sp, #0x20 242dc0: a9401404 ldp x4, x5, [x0] 242dc4: a9000c02 stp x2, x3, [x0] 242dc8: a9410c02 ldp x2, x3, [x0, #16] 242dcc: a9411c26 ldp x6, x7, [x1, #16] 242dd0: a9011c06 stp x6, x7, [x0, #16] 242dd4: a9001424 stp x4, x5, [x1] 242dd8: a9010c22 stp x2, x3, [x1, #16] 242ddc: 910083ff add sp, sp, #0x20 242de0: d65f03c0 ret 242de4: d503201f nop > > So at best this feels some micro-optimization. But then, this is only > call a limited number of time at each domain build. Is it really worth > it? It's not hot path here. Not sure about worth. Personally i will try my best to do things well according to my understanding. > > On a side note, I have noticed you are sending a lot of > optimization/clean-up patch. What is your end goal here? My goal is to understand how xen works well. > > If it is to improve the performance, then there are much bigger fish > to fry within Xen code base. I am happy to point some of them based on > where you are looking to improve. Surly i want to improve performance. Features like Fast Startup ( I learned from xen summit 2018, samsung automotive presentation). But currently i don't understand xen well, only a few weeks experience. I'm afraid can't catch big fish. > > Cheers, >
Hi, On 12/06/2019 11:08, chenbaodong wrote: > > On 6/12/19 17:08, Julien Grall wrote: >> Hi, >> >> On 6/12/19 6:42 AM, Baodong Chen wrote: >>> Swap function can be used when calling sort(). >>> or else, the default swap function generic_swap() is used, >>> which is a little inefficient. >> >> I am not entirely convince this will be more efficient. mmio_handler does not >> fit in 64 bit, so the compiler may decide to do either multiple load or >> replace with a memcpy. > > Hello Julien, > > I have checked the disassemble result, > > and IIUC generic_swap has a loop so it should be a little inefficient. I'm not > expert about hardware, please correct me if i'm wrong. I am not an hardware expert too... But as I pointed out below this is a micro-optimization. In other words, you are tailoring a specific function that may run faster now, but this is improvement is going to be lost as this is just a very tiny part of the domain creation. [...] >> >> So at best this feels some micro-optimization. But then, this is only call a >> limited number of time at each domain build. Is it really worth it? > > It's not hot path here. > > Not sure about worth. > > Personally i will try my best to do things well according to my understanding. Micro-optimization are always good, but you also have to factor the cost of maintaining and whether this will improve significantly Xen. > >> >> On a side note, I have noticed you are sending a lot of optimization/clean-up >> patch. What is your end goal here? > > My goal is to understand how xen works well. > >> >> If it is to improve the performance, then there are much bigger fish to fry >> within Xen code base. I am happy to point some of them based on where you are >> looking to improve. > > Surly i want to improve performance. > > Features like Fast Startup ( I learned from xen summit 2018, samsung automotive > presentation). > > But currently i don't understand xen well, only a few weeks experience. We do have small task for newcomers that would improve Xen code base and also allow your to understand more some part of the code. If you have a specific area of interest, I can see if I have some small tasks there. Cheers,
On 6/12/19 20:21, Julien Grall wrote: > Hi, > > On 12/06/2019 11:08, chenbaodong wrote: >> >> On 6/12/19 17:08, Julien Grall wrote: >>> Hi, >>> >>> On 6/12/19 6:42 AM, Baodong Chen wrote: >>>> Swap function can be used when calling sort(). >>>> or else, the default swap function generic_swap() is used, >>>> which is a little inefficient. >>> >>> I am not entirely convince this will be more efficient. mmio_handler >>> does not fit in 64 bit, so the compiler may decide to do either >>> multiple load or replace with a memcpy. >> >> Hello Julien, >> >> I have checked the disassemble result, >> >> and IIUC generic_swap has a loop so it should be a little >> inefficient. I'm not expert about hardware, please correct me if i'm >> wrong. > > I am not an hardware expert too... But as I pointed out below this is > a micro-optimization. In other words, you are tailoring a specific > function that may run faster now, but this is improvement is going to > be lost as this is just a very tiny part of the domain creation. > > [...] > >>> >>> So at best this feels some micro-optimization. But then, this is >>> only call a limited number of time at each domain build. Is it >>> really worth it? >> >> It's not hot path here. >> >> Not sure about worth. >> >> Personally i will try my best to do things well according to my >> understanding. > > Micro-optimization are always good, but you also have to factor the > cost of maintaining and whether this will improve significantly Xen. > >> >>> >>> On a side note, I have noticed you are sending a lot of >>> optimization/clean-up patch. What is your end goal here? >> >> My goal is to understand how xen works well. >> >>> >>> If it is to improve the performance, then there are much bigger fish >>> to fry within Xen code base. I am happy to point some of them based >>> on where you are looking to improve. >> >> Surly i want to improve performance. >> >> Features like Fast Startup ( I learned from xen summit 2018, samsung >> automotive presentation). >> >> But currently i don't understand xen well, only a few weeks experience. > > We do have small task for newcomers that would improve Xen code base > and also allow your to understand more some part of the code. > > If you have a specific area of interest, I can see if I have some > small tasks there. I'm happy with this. Interested in arm platform for embedded and automotive use case. things like in this link: https://xenproject.org/developers/teams/embedded-and-automotive/ > > Cheers, >
On Thu, 13 Jun 2019, chenbaodong wrote: > > > But currently i don't understand xen well, only a few weeks experience. > > > > We do have small task for newcomers that would improve Xen code base and > > also allow your to understand more some part of the code. > > > > If you have a specific area of interest, I can see if I have some small > > tasks there. > > I'm happy with this. > > Interested in arm platform for embedded and automotive use case. > > things like in this link: > https://xenproject.org/developers/teams/embedded-and-automotive/ Hi Baodong, Would you be up for a documentation task? Don't worry if the English is not perfect, it can easily be adjusted on commit, as long as the content is correct. We have recently started an effort to write better documentation for all external Xen interfaces. I wrote a list of these interfaces that need documenting: https://lists.xenproject.org/archives/html/xen-devel/2019-06/msg01111.html For instance, we are currently missing a document describing in details the device tree exposed to a DomU. You can see the code that builds such a device tree in tools/libxl//libxl_arm.c, for normal guests, and in xen/arch/arm/domain_build.c for dom0 and dom0less guests. From within a guest, you can see the devicetree by accessing /proc/device-tree (if the guest is Linux). If you are up for it, it would be great to get that written down clearly. You could add to docs/misc/arm/device-tree/guest.txt, which is only describing the xen hypervisor node, but nothing else. Probably it would be better to rename docs/misc/arm/device-tree/guest.txt to docs/misc/arm/device-tree/hypervisor.txt and create a new docs/misc/arm/device-tree/guest.txt document with the description of all nodes exposed to domUs. You could point to the full description of each node to the binding under Linux. For instance, the binding for the Arm arch timer is described here: https://lists.xenproject.org/archives/html/xen-devel/2019-06/msg01111.html Does it make sense?
On Mon, 24 Jun 2019, Stefano Stabellini wrote: > On Thu, 13 Jun 2019, chenbaodong wrote: > > > > But currently i don't understand xen well, only a few weeks experience. > > > > > > We do have small task for newcomers that would improve Xen code base and > > > also allow your to understand more some part of the code. > > > > > > If you have a specific area of interest, I can see if I have some small > > > tasks there. > > > > I'm happy with this. > > > > Interested in arm platform for embedded and automotive use case. > > > > things like in this link: > > https://xenproject.org/developers/teams/embedded-and-automotive/ > > Hi Baodong, > > Would you be up for a documentation task? Don't worry if the English is > not perfect, it can easily be adjusted on commit, as long as the content > is correct. > > We have recently started an effort to write better documentation for all > external Xen interfaces. I wrote a list of these interfaces that need > documenting: > > https://lists.xenproject.org/archives/html/xen-devel/2019-06/msg01111.html > > > For instance, we are currently missing a document describing in details > the device tree exposed to a DomU. You can see the code that builds such > a device tree in tools/libxl//libxl_arm.c, for normal guests, and in > xen/arch/arm/domain_build.c for dom0 and dom0less guests. From within a > guest, you can see the devicetree by accessing /proc/device-tree (if the > guest is Linux). > > If you are up for it, it would be great to get that written down > clearly. You could add to docs/misc/arm/device-tree/guest.txt, which is > only describing the xen hypervisor node, but nothing else. Probably it > would be better to rename docs/misc/arm/device-tree/guest.txt to > docs/misc/arm/device-tree/hypervisor.txt and create a new > docs/misc/arm/device-tree/guest.txt document with the description of all > nodes exposed to domUs. You could point to the full description of each > node to the binding under Linux. For instance, the binding for the Arm > arch timer is described here: > https://lists.xenproject.org/archives/html/xen-devel/2019-06/msg01111.html > > Does it make sense? Let me add that if you prefer to document one of the other interfaces listed above in my email, you are welcome to pick another one. For example, we are also missing a doc about the DomU memory map, listing all memory regions with addresses and sizes, both MMIO and normal memory. For that, most of the information is: xen/include/public/arch-arm.h A well written in-code comment in arch-arm.h would be OK, or also a document under docs/misc/arm.
Hi Stefano, On 24/06/2019 19:27, Stefano Stabellini wrote: > On Mon, 24 Jun 2019, Stefano Stabellini wrote: >> On Thu, 13 Jun 2019, chenbaodong wrote: > Let me add that if you prefer to document one of the other interfaces > listed above in my email, you are welcome to pick another one. For > example, we are also missing a doc about the DomU memory map, listing > all memory regions with addresses and sizes, both MMIO and normal > memory. For that, most of the information is: > > xen/include/public/arch-arm.h > > A well written in-code comment in arch-arm.h would be OK, or also a > document under docs/misc/arm. Please no duplication, it is already quite hard to maintain one place. Instead, we should document all the headers in a documented format that can be extracted automatically. Cheers,
On Mon, 24 Jun 2019, Julien Grall wrote: > Hi Stefano, > > On 24/06/2019 19:27, Stefano Stabellini wrote: > > On Mon, 24 Jun 2019, Stefano Stabellini wrote: > >> On Thu, 13 Jun 2019, chenbaodong wrote: > > Let me add that if you prefer to document one of the other interfaces > > listed above in my email, you are welcome to pick another one. For > > example, we are also missing a doc about the DomU memory map, listing > > all memory regions with addresses and sizes, both MMIO and normal > > memory. For that, most of the information is: > > > > xen/include/public/arch-arm.h > > > > A well written in-code comment in arch-arm.h would be OK, or also a > > document under docs/misc/arm. > > Please no duplication, it is already quite hard to maintain one place. > Instead, we should document all the headers in a documented format that > can be extracted automatically. As we have no such thing today (as far as I am aware), please make a proposal with a bit more details, otherwise I don't think Baodong will be able to take the next step.
Hi, On 6/24/19 9:17 PM, Stefano Stabellini wrote: > On Mon, 24 Jun 2019, Julien Grall wrote: >> Hi Stefano, >> >> On 24/06/2019 19:27, Stefano Stabellini wrote: >>> On Mon, 24 Jun 2019, Stefano Stabellini wrote: >>>> On Thu, 13 Jun 2019, chenbaodong wrote: >>> Let me add that if you prefer to document one of the other interfaces >>> listed above in my email, you are welcome to pick another one. For >>> example, we are also missing a doc about the DomU memory map, listing >>> all memory regions with addresses and sizes, both MMIO and normal >>> memory. For that, most of the information is: >>> >>> xen/include/public/arch-arm.h >>> >>> A well written in-code comment in arch-arm.h would be OK, or also a >>> document under docs/misc/arm. >> >> Please no duplication, it is already quite hard to maintain one place. >> Instead, we should document all the headers in a documented format that >> can be extracted automatically. > > As we have no such thing today (as far as I am aware), please make a > proposal with a bit more details, otherwise I don't think Baodong will > be able to take the next step. I don't have a concrete proposal so far. Except that documentation outside of the headers is a no-go from my side. The goal of documenting within the headers rather than outside is you also help the developer of guest OS. A few weeks ago Ian Jackson pointed to docs/xen-headers for a potential syntax. Sadly, there are no documentation of the script so far. I haven't had time to look it so far. Cheers,
On Mon, 24 Jun 2019, Julien Grall wrote: > Hi, > > On 6/24/19 9:17 PM, Stefano Stabellini wrote: > > On Mon, 24 Jun 2019, Julien Grall wrote: > > > Hi Stefano, > > > > > > On 24/06/2019 19:27, Stefano Stabellini wrote: > > > > On Mon, 24 Jun 2019, Stefano Stabellini wrote: > > > > > On Thu, 13 Jun 2019, chenbaodong wrote: > > > > Let me add that if you prefer to document one of the other interfaces > > > > listed above in my email, you are welcome to pick another one. For > > > > example, we are also missing a doc about the DomU memory map, listing > > > > all memory regions with addresses and sizes, both MMIO and normal > > > > memory. For that, most of the information is: > > > > > > > > xen/include/public/arch-arm.h > > > > > > > > A well written in-code comment in arch-arm.h would be OK, or also a > > > > document under docs/misc/arm. > > > > > > Please no duplication, it is already quite hard to maintain one place. > > > Instead, we should document all the headers in a documented format that > > > can be extracted automatically. > > > > As we have no such thing today (as far as I am aware), please make a > > proposal with a bit more details, otherwise I don't think Baodong will > > be able to take the next step. > > I don't have a concrete proposal so far. Except that documentation outside of > the headers is a no-go from my side. The goal of documenting within the > headers rather than outside is you also help the developer of guest OS. > > A few weeks ago Ian Jackson pointed to docs/xen-headers for a potential > syntax. Sadly, there are no documentation of the script so far. I haven't had > time to look it so far. In that case, I'd suggest for Baodong to either pick the device tree documentation item (assuming you are OK with that one being under docs/misc/arm) or just write a normal in-code comment in arch-arm.h for the domU memory map not worrying about the format of the in-code comment for now.
HiStefano, On 25/06/2019 00:59, Stefano Stabellini wrote: > On Mon, 24 Jun 2019, Julien Grall wrote: >> Hi, >> >> On 6/24/19 9:17 PM, Stefano Stabellini wrote: >>> On Mon, 24 Jun 2019, Julien Grall wrote: >>>> Hi Stefano, >>>> >>>> On 24/06/2019 19:27, Stefano Stabellini wrote: >>>>> On Mon, 24 Jun 2019, Stefano Stabellini wrote: >>>>>> On Thu, 13 Jun 2019, chenbaodong wrote: >>>>> Let me add that if you prefer to document one of the other interfaces >>>>> listed above in my email, you are welcome to pick another one. For >>>>> example, we are also missing a doc about the DomU memory map, listing >>>>> all memory regions with addresses and sizes, both MMIO and normal >>>>> memory. For that, most of the information is: >>>>> >>>>> xen/include/public/arch-arm.h >>>>> >>>>> A well written in-code comment in arch-arm.h would be OK, or also a >>>>> document under docs/misc/arm. >>>> >>>> Please no duplication, it is already quite hard to maintain one place. >>>> Instead, we should document all the headers in a documented format that >>>> can be extracted automatically. >>> >>> As we have no such thing today (as far as I am aware), please make a >>> proposal with a bit more details, otherwise I don't think Baodong will >>> be able to take the next step. >> >> I don't have a concrete proposal so far. Except that documentation outside of >> the headers is a no-go from my side. The goal of documenting within the >> headers rather than outside is you also help the developer of guest OS. >> >> A few weeks ago Ian Jackson pointed to docs/xen-headers for a potential >> syntax. Sadly, there are no documentation of the script so far. I haven't had >> time to look it so far. > > In that case, I'd suggest for Baodong to either pick the device tree > documentation item (assuming you are OK with that one being under > docs/misc/arm) or just write a normal in-code comment in arch-arm.h for > the domU memory map not worrying about the format of the in-code comment > for now. I don't think we have specific place for documenting device-tree so docs/misc/arm would be suitable. Regarding in-code comment in arch-arm.h This will always be an improvement to what we have. However, it would be good if someone take an action to formalize the documentation format. Cheers,
On 6/25/19 16:46, Julien Grall wrote: > HiStefano, > > On 25/06/2019 00:59, Stefano Stabellini wrote: >> On Mon, 24 Jun 2019, Julien Grall wrote: >>> Hi, >>> >>> On 6/24/19 9:17 PM, Stefano Stabellini wrote: >>>> On Mon, 24 Jun 2019, Julien Grall wrote: >>>>> Hi Stefano, >>>>> >>>>> On 24/06/2019 19:27, Stefano Stabellini wrote: >>>>>> On Mon, 24 Jun 2019, Stefano Stabellini wrote: >>>>>>> On Thu, 13 Jun 2019, chenbaodong wrote: >>>>>> Let me add that if you prefer to document one of the other >>>>>> interfaces >>>>>> listed above in my email, you are welcome to pick another one. For >>>>>> example, we are also missing a doc about the DomU memory map, >>>>>> listing >>>>>> all memory regions with addresses and sizes, both MMIO and normal >>>>>> memory. For that, most of the information is: >>>>>> >>>>>> xen/include/public/arch-arm.h >>>>>> >>>>>> A well written in-code comment in arch-arm.h would be OK, or also a >>>>>> document under docs/misc/arm. >>>>> >>>>> Please no duplication, it is already quite hard to maintain one >>>>> place. >>>>> Instead, we should document all the headers in a documented format >>>>> that >>>>> can be extracted automatically. >>>> >>>> As we have no such thing today (as far as I am aware), please make a >>>> proposal with a bit more details, otherwise I don't think Baodong will >>>> be able to take the next step. >>> >>> I don't have a concrete proposal so far. Except that documentation >>> outside of >>> the headers is a no-go from my side. The goal of documenting within the >>> headers rather than outside is you also help the developer of guest OS. >>> >>> A few weeks ago Ian Jackson pointed to docs/xen-headers for a potential >>> syntax. Sadly, there are no documentation of the script so far. I >>> haven't had >>> time to look it so far. >> >> In that case, I'd suggest for Baodong to either pick the device tree >> documentation item (assuming you are OK with that one being under >> docs/misc/arm) or just write a normal in-code comment in arch-arm.h for >> the domU memory map not worrying about the format of the in-code comment >> for now. > > I don't think we have specific place for documenting device-tree so > docs/misc/arm would be suitable. > > Regarding in-code comment in arch-arm.h This will always be an > improvement to what we have. However, it would be good if someone take > an action to formalize the documentation format. I will look into this. Currently interrupted by some other work, will be back soon. > > Cheers, >
diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c index ae7ef96..5ce7dc4 100644 --- a/xen/arch/arm/io.c +++ b/xen/arch/arm/io.c @@ -92,6 +92,17 @@ static int cmp_mmio_handler(const void *key, const void *elem) return 0; } +static void swap_mmio_handler(void *a, void *b, int size) +{ + struct mmio_handler *handler0 = a; + struct mmio_handler *handler1 = b; + struct mmio_handler tmp; + + tmp = *handler0; + *handler0 = *handler1; + *handler1 = tmp; +} + static const struct mmio_handler *find_mmio_handler(struct domain *d, paddr_t gpa) { @@ -174,7 +185,7 @@ void register_mmio_handler(struct domain *d, /* Sort mmio handlers in ascending order based on base address */ sort(vmmio->handlers, vmmio->num_entries, sizeof(struct mmio_handler), - cmp_mmio_handler, NULL); + cmp_mmio_handler, swap_mmio_handler); write_unlock(&vmmio->lock); }
Swap function can be used when calling sort(). or else, the default swap function generic_swap() is used, which is a little inefficient. Signed-off-by: Baodong Chen <chenbaodong@mxnavi.com> --- xen/arch/arm/io.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)