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

Baodong Chen posted 1 patch 4 years, 10 months 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 years, 10 months 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 years, 10 months 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 years, 10 months 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 years, 10 months 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 4 years, 10 months 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
Re: [Xen-devel] [PATCH] xen/arm: io: add function swap_mmio_handler()
Posted by Stefano Stabellini 4 years, 10 months ago
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?

_______________________________________________
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 Stefano Stabellini 4 years, 10 months ago
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.

_______________________________________________
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 years, 10 months ago
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,

-- 
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 Stefano Stabellini 4 years, 10 months ago
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.

_______________________________________________
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 years, 10 months ago
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,

-- 
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 Stefano Stabellini 4 years, 10 months ago
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.

_______________________________________________
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 years, 10 months ago
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,

-- 
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 years, 10 months ago
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,
>

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