* [PATCH for-4.12] x86/p2m: fix p2m_finish_type_change()
@ 2019-01-09 10:41 Razvan Cojocaru
2019-01-09 11:16 ` Jan Beulich
2019-01-10 23:42 ` George Dunlap
0 siblings, 2 replies; 4+ messages in thread
From: Razvan Cojocaru @ 2019-01-09 10:41 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, Razvan Cojocaru, george.dunlap, andrew.cooper3,
jbeulich, roger.pau
finish_type_change() returns a negative int on error, but the
current code checks if ( !rc ). Also properly indent the out:
label while at it.
Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
---
xen/arch/x86/mm/p2m.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 5451f16..e08f6b1 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1176,7 +1176,7 @@ int p2m_finish_type_change(struct domain *d,
rc = finish_type_change(hostp2m, first_gfn, max_nr);
- if ( !rc )
+ if ( rc < 0 )
goto out;
#ifdef CONFIG_HVM
@@ -1193,13 +1193,13 @@ int p2m_finish_type_change(struct domain *d,
rc = finish_type_change(altp2m, first_gfn, max_nr);
p2m_unlock(altp2m);
- if ( !rc )
+ if ( rc < 0 )
goto out;
}
}
#endif
-out:
+ out:
p2m_unlock(hostp2m);
return rc;
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH for-4.12] x86/p2m: fix p2m_finish_type_change()
2019-01-09 10:41 [PATCH for-4.12] x86/p2m: fix p2m_finish_type_change() Razvan Cojocaru
@ 2019-01-09 11:16 ` Jan Beulich
2019-01-10 23:48 ` George Dunlap
2019-01-10 23:42 ` George Dunlap
1 sibling, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2019-01-09 11:16 UTC (permalink / raw)
To: Razvan Cojocaru
Cc: George Dunlap, Andrew Cooper, Wei Liu, xen-devel, Roger Pau Monne
>>> On 09.01.19 at 11:41, <rcojocaru@bitdefender.com> wrote:
> finish_type_change() returns a negative int on error, but the
> current code checks if ( !rc ).
For the purpose of determining the backporting scope (none here)
it would be nice if in such a case you could point out the commit
introducing the breakage.
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1176,7 +1176,7 @@ int p2m_finish_type_change(struct domain *d,
>
> rc = finish_type_change(hostp2m, first_gfn, max_nr);
>
> - if ( !rc )
> + if ( rc < 0 )
> goto out;
>
> #ifdef CONFIG_HVM
> @@ -1193,13 +1193,13 @@ int p2m_finish_type_change(struct domain *d,
> rc = finish_type_change(altp2m, first_gfn, max_nr);
> p2m_unlock(altp2m);
>
> - if ( !rc )
> + if ( rc < 0 )
> goto out;
> }
> }
> #endif
May I ask that you also fix the other return value issue here: Either
we mean to indicate to the caller the 0/1 distinction (in which case
it should be correct, i.e. cumulative across all finish_type_change()
calls made here), which was the behavior before 0fb4b58c8b
("x86/altp2m: fix display frozen when switching to a new view early"),
or we mean to signal success to the caller only by handing back zero.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH for-4.12] x86/p2m: fix p2m_finish_type_change()
2019-01-09 10:41 [PATCH for-4.12] x86/p2m: fix p2m_finish_type_change() Razvan Cojocaru
2019-01-09 11:16 ` Jan Beulich
@ 2019-01-10 23:42 ` George Dunlap
1 sibling, 0 replies; 4+ messages in thread
From: George Dunlap @ 2019-01-10 23:42 UTC (permalink / raw)
To: Razvan Cojocaru
Cc: Wei Liu, Andrew Cooper, George Dunlap, Jan Beulich, xen-devel,
Roger Pau Monne
> On Jan 9, 2019, at 8:41 PM, Razvan Cojocaru <rcojocaru@bitdefender.com> wrote:
>
> finish_type_change() returns a negative int on error, but the
> current code checks if ( !rc ). Also properly indent the out:
> label while at it.
>
> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH for-4.12] x86/p2m: fix p2m_finish_type_change()
2019-01-09 11:16 ` Jan Beulich
@ 2019-01-10 23:48 ` George Dunlap
0 siblings, 0 replies; 4+ messages in thread
From: George Dunlap @ 2019-01-10 23:48 UTC (permalink / raw)
To: Jan Beulich
Cc: Wei Liu, Razvan Cojocaru, Andrew Cooper, George Dunlap,
Paul Durrant, Anthony Perard, xen-devel, Roger Pau Monne
> On Jan 9, 2019, at 9:16 PM, Jan Beulich <jbeulich@suse.com> wrote:
>
>>>> On 09.01.19 at 11:41, <rcojocaru@bitdefender.com> wrote:
>> finish_type_change() returns a negative int on error, but the
>> current code checks if ( !rc ).
>
> For the purpose of determining the backporting scope (none here)
> it would be nice if in such a case you could point out the commit
> introducing the breakage.
>
>> --- a/xen/arch/x86/mm/p2m.c
>> +++ b/xen/arch/x86/mm/p2m.c
>> @@ -1176,7 +1176,7 @@ int p2m_finish_type_change(struct domain *d,
>>
>> rc = finish_type_change(hostp2m, first_gfn, max_nr);
>>
>> - if ( !rc )
>> + if ( rc < 0 )
>> goto out;
>>
>> #ifdef CONFIG_HVM
>> @@ -1193,13 +1193,13 @@ int p2m_finish_type_change(struct domain *d,
>> rc = finish_type_change(altp2m, first_gfn, max_nr);
>> p2m_unlock(altp2m);
>>
>> - if ( !rc )
>> + if ( rc < 0 )
>> goto out;
>> }
>> }
>> #endif
>
> May I ask that you also fix the other return value issue here: Either
> we mean to indicate to the caller the 0/1 distinction (in which case
> it should be correct, i.e. cumulative across all finish_type_change()
> calls made here), which was the behavior before 0fb4b58c8b
> ("x86/altp2m: fix display frozen when switching to a new view early"),
> or we mean to signal success to the caller only by handing back zero.
The “root” caller of p2m_finish_type_change() is XEN_DMOP_map_mem_type_to_ioreq_server; It doesn’t look like it does anything useful with positive values, and it’s not clear from an interface perspective that returning a positive value from that hypercall makes any sense.
Perhaps it would be best to just discard the positive values for now.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-01-10 23:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-09 10:41 [PATCH for-4.12] x86/p2m: fix p2m_finish_type_change() Razvan Cojocaru
2019-01-09 11:16 ` Jan Beulich
2019-01-10 23:48 ` George Dunlap
2019-01-10 23:42 ` George Dunlap
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.