[Xen-devel] [PATCH] xen/arm: io: add function swap_mmio_handler()

Baodong Chen posted 1 patch 4 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/1560318154-19095-1-git-send-email-chenbaodong@mxnavi.com
xen/arch/arm/io.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)

[Xen-devel] [PATCH] xen/arm: io: add function swap_mmio_handler()

Posted by Baodong Chen 4 days ago
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(-)

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);
 }
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen/arm: io: add function swap_mmio_handler()

Posted by Julien Grall 4 days ago
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,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen/arm: io: add function swap_mmio_handler()

Posted by chenbaodong 4 days ago
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,
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen/arm: io: add function swap_mmio_handler()

Posted by Julien Grall 4 days ago
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,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen/arm: io: add function swap_mmio_handler()

Posted by chenbaodong 3 days ago
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,
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel