* [Qemu-devel] [PATCH v3] exec.c: refactor function flatview_add_to_dispatch()
@ 2019-03-11 5:42 Wei Yang
2019-03-11 13:42 ` Paolo Bonzini
0 siblings, 1 reply; 3+ messages in thread
From: Wei Yang @ 2019-03-11 5:42 UTC (permalink / raw)
To: qemu-devel; +Cc: rth, pbonzini, Wei Yang
flatview_add_to_dispatch() registers page based on the condition of
*section*, which may looks like this:
|s|PPPPPPP|s|
where s stands for subpage and P for page.
The procedure of this function could be described as:
- register first subpage
- register page
- register last subpage
This means the procedure could be simplified into these three steps
instead of a loop iteration.
This patch refactors the function into three corresponding steps and
adds some comment to clarify it.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
v3:
* pass int128_make64() instead of 0 to int128_gt()
v2:
* removes the loop iteration as suggested by Paolo
---
exec.c | 50 +++++++++++++++++++++++++++++++++-----------------
1 file changed, 33 insertions(+), 17 deletions(-)
diff --git a/exec.c b/exec.c
index 0959b00c06..79cd561b3b 100644
--- a/exec.c
+++ b/exec.c
@@ -1598,34 +1598,50 @@ static void register_multipage(FlatView *fv,
phys_page_set(d, start_addr >> TARGET_PAGE_BITS, num_pages, section_index);
}
+/*
+ * The range in *section* may look like this:
+ *
+ * |s|PPPPPPP|s|
+ *
+ * where s stands for subpage and P for page.
+ *
+ * The procedure in following function could be described as:
+ *
+ * - register first subpage
+ * - register page
+ * - register last subpage
+ */
void flatview_add_to_dispatch(FlatView *fv, MemoryRegionSection *section)
{
- MemoryRegionSection now = *section, remain = *section;
+ MemoryRegionSection now, remain = *section;
Int128 page_size = int128_make64(TARGET_PAGE_SIZE);
- if (now.offset_within_address_space & ~TARGET_PAGE_MASK) {
- uint64_t left = TARGET_PAGE_ALIGN(now.offset_within_address_space)
- - now.offset_within_address_space;
+ /* register first subpage */
+ if (remain.offset_within_address_space & ~TARGET_PAGE_MASK) {
+ uint64_t left = TARGET_PAGE_ALIGN(remain.offset_within_address_space)
+ - remain.offset_within_address_space;
+ now = remain;
now.size = int128_min(int128_make64(left), now.size);
+ remain.size = int128_sub(remain.size, now.size);
+ remain.offset_within_address_space += int128_get64(now.size);
+ remain.offset_within_region += int128_get64(now.size);
register_subpage(fv, &now);
- } else {
- now.size = int128_zero();
}
- while (int128_ne(remain.size, now.size)) {
+
+ /* register page */
+ if (int128_ge(remain.size, page_size)) {
+ now = remain;
+ now.size = int128_and(now.size, int128_neg(page_size));
remain.size = int128_sub(remain.size, now.size);
remain.offset_within_address_space += int128_get64(now.size);
remain.offset_within_region += int128_get64(now.size);
- now = remain;
- if (int128_lt(remain.size, page_size)) {
- register_subpage(fv, &now);
- } else if (remain.offset_within_address_space & ~TARGET_PAGE_MASK) {
- now.size = page_size;
- register_subpage(fv, &now);
- } else {
- now.size = int128_and(now.size, int128_neg(page_size));
- register_multipage(fv, &now);
- }
+ register_multipage(fv, &now);
+ }
+
+ /* register last subpage */
+ if (int128_gt(remain.size, int128_make64(0))) {
+ register_subpage(fv, &remain);
}
}
--
2.19.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH v3] exec.c: refactor function flatview_add_to_dispatch()
2019-03-11 5:42 [Qemu-devel] [PATCH v3] exec.c: refactor function flatview_add_to_dispatch() Wei Yang
@ 2019-03-11 13:42 ` Paolo Bonzini
2019-03-11 22:19 ` Wei Yang
0 siblings, 1 reply; 3+ messages in thread
From: Paolo Bonzini @ 2019-03-11 13:42 UTC (permalink / raw)
To: Wei Yang, qemu-devel; +Cc: rth
On 11/03/19 06:42, Wei Yang wrote:
> flatview_add_to_dispatch() registers page based on the condition of
> *section*, which may looks like this:
>
> |s|PPPPPPP|s|
>
> where s stands for subpage and P for page.
>
> The procedure of this function could be described as:
>
> - register first subpage
> - register page
> - register last subpage
>
> This means the procedure could be simplified into these three steps
> instead of a loop iteration.
>
> This patch refactors the function into three corresponding steps and
> adds some comment to clarify it.
>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>
> ---
> v3:
> * pass int128_make64() instead of 0 to int128_gt()
> v2:
> * removes the loop iteration as suggested by Paolo
> ---
> exec.c | 50 +++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 33 insertions(+), 17 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 0959b00c06..79cd561b3b 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1598,34 +1598,50 @@ static void register_multipage(FlatView *fv,
> phys_page_set(d, start_addr >> TARGET_PAGE_BITS, num_pages, section_index);
> }
>
> +/*
> + * The range in *section* may look like this:
> + *
> + * |s|PPPPPPP|s|
> + *
> + * where s stands for subpage and P for page.
> + *
> + * The procedure in following function could be described as:
> + *
> + * - register first subpage
> + * - register page
> + * - register last subpage
The last paragraph is a duplicate of the comments in the function, so it
can be deleted. I did that and queued the patch.
Paolo
> + */
> void flatview_add_to_dispatch(FlatView *fv, MemoryRegionSection *section)
> {
> - MemoryRegionSection now = *section, remain = *section;
> + MemoryRegionSection now, remain = *section;
> Int128 page_size = int128_make64(TARGET_PAGE_SIZE);
>
> - if (now.offset_within_address_space & ~TARGET_PAGE_MASK) {
> - uint64_t left = TARGET_PAGE_ALIGN(now.offset_within_address_space)
> - - now.offset_within_address_space;
> + /* register first subpage */
> + if (remain.offset_within_address_space & ~TARGET_PAGE_MASK) {
> + uint64_t left = TARGET_PAGE_ALIGN(remain.offset_within_address_space)
> + - remain.offset_within_address_space;
>
> + now = remain;
> now.size = int128_min(int128_make64(left), now.size);
> + remain.size = int128_sub(remain.size, now.size);
> + remain.offset_within_address_space += int128_get64(now.size);
> + remain.offset_within_region += int128_get64(now.size);
> register_subpage(fv, &now);
> - } else {
> - now.size = int128_zero();
> }
> - while (int128_ne(remain.size, now.size)) {
> +
> + /* register page */
> + if (int128_ge(remain.size, page_size)) {
> + now = remain;
> + now.size = int128_and(now.size, int128_neg(page_size));
> remain.size = int128_sub(remain.size, now.size);
> remain.offset_within_address_space += int128_get64(now.size);
> remain.offset_within_region += int128_get64(now.size);
> - now = remain;
> - if (int128_lt(remain.size, page_size)) {
> - register_subpage(fv, &now);
> - } else if (remain.offset_within_address_space & ~TARGET_PAGE_MASK) {
> - now.size = page_size;
> - register_subpage(fv, &now);
> - } else {
> - now.size = int128_and(now.size, int128_neg(page_size));
> - register_multipage(fv, &now);
> - }
> + register_multipage(fv, &now);
> + }
> +
> + /* register last subpage */
> + if (int128_gt(remain.size, int128_make64(0))) {
> + register_subpage(fv, &remain);
> }
> }
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH v3] exec.c: refactor function flatview_add_to_dispatch()
2019-03-11 13:42 ` Paolo Bonzini
@ 2019-03-11 22:19 ` Wei Yang
0 siblings, 0 replies; 3+ messages in thread
From: Wei Yang @ 2019-03-11 22:19 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Wei Yang, qemu-devel, rth
On Mon, Mar 11, 2019 at 02:42:58PM +0100, Paolo Bonzini wrote:
>On 11/03/19 06:42, Wei Yang wrote:
>> flatview_add_to_dispatch() registers page based on the condition of
>> *section*, which may looks like this:
>>
>> |s|PPPPPPP|s|
>>
>> where s stands for subpage and P for page.
>>
>> The procedure of this function could be described as:
>>
>> - register first subpage
>> - register page
>> - register last subpage
>>
>> This means the procedure could be simplified into these three steps
>> instead of a loop iteration.
>>
>> This patch refactors the function into three corresponding steps and
>> adds some comment to clarify it.
>>
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>>
>> ---
>> v3:
>> * pass int128_make64() instead of 0 to int128_gt()
>> v2:
>> * removes the loop iteration as suggested by Paolo
>> ---
>> exec.c | 50 +++++++++++++++++++++++++++++++++-----------------
>> 1 file changed, 33 insertions(+), 17 deletions(-)
>>
>> diff --git a/exec.c b/exec.c
>> index 0959b00c06..79cd561b3b 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -1598,34 +1598,50 @@ static void register_multipage(FlatView *fv,
>> phys_page_set(d, start_addr >> TARGET_PAGE_BITS, num_pages, section_index);
>> }
>>
>> +/*
>> + * The range in *section* may look like this:
>> + *
>> + * |s|PPPPPPP|s|
>> + *
>> + * where s stands for subpage and P for page.
>> + *
>> + * The procedure in following function could be described as:
>> + *
>> + * - register first subpage
>> + * - register page
>> + * - register last subpage
>
>The last paragraph is a duplicate of the comments in the function, so it
>can be deleted. I did that and queued the patch.
>
Thanks :-)
>Paolo
>
>> + */
>> void flatview_add_to_dispatch(FlatView *fv, MemoryRegionSection *section)
>> {
>> - MemoryRegionSection now = *section, remain = *section;
>> + MemoryRegionSection now, remain = *section;
>> Int128 page_size = int128_make64(TARGET_PAGE_SIZE);
>>
>> - if (now.offset_within_address_space & ~TARGET_PAGE_MASK) {
>> - uint64_t left = TARGET_PAGE_ALIGN(now.offset_within_address_space)
>> - - now.offset_within_address_space;
>> + /* register first subpage */
>> + if (remain.offset_within_address_space & ~TARGET_PAGE_MASK) {
>> + uint64_t left = TARGET_PAGE_ALIGN(remain.offset_within_address_space)
>> + - remain.offset_within_address_space;
>>
>> + now = remain;
>> now.size = int128_min(int128_make64(left), now.size);
>> + remain.size = int128_sub(remain.size, now.size);
>> + remain.offset_within_address_space += int128_get64(now.size);
>> + remain.offset_within_region += int128_get64(now.size);
>> register_subpage(fv, &now);
>> - } else {
>> - now.size = int128_zero();
>> }
>> - while (int128_ne(remain.size, now.size)) {
>> +
>> + /* register page */
>> + if (int128_ge(remain.size, page_size)) {
>> + now = remain;
>> + now.size = int128_and(now.size, int128_neg(page_size));
>> remain.size = int128_sub(remain.size, now.size);
>> remain.offset_within_address_space += int128_get64(now.size);
>> remain.offset_within_region += int128_get64(now.size);
>> - now = remain;
>> - if (int128_lt(remain.size, page_size)) {
>> - register_subpage(fv, &now);
>> - } else if (remain.offset_within_address_space & ~TARGET_PAGE_MASK) {
>> - now.size = page_size;
>> - register_subpage(fv, &now);
>> - } else {
>> - now.size = int128_and(now.size, int128_neg(page_size));
>> - register_multipage(fv, &now);
>> - }
>> + register_multipage(fv, &now);
>> + }
>> +
>> + /* register last subpage */
>> + if (int128_gt(remain.size, int128_make64(0))) {
>> + register_subpage(fv, &remain);
>> }
>> }
>>
>>
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-03-11 22:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-11 5:42 [Qemu-devel] [PATCH v3] exec.c: refactor function flatview_add_to_dispatch() Wei Yang
2019-03-11 13:42 ` Paolo Bonzini
2019-03-11 22:19 ` Wei Yang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.