Holding a valid struct page_info * in hands already means the referenced
MFN is valid; there's no need to check that again. Convert the checking
logic to a switch(), to help keeping the extra (and questionable) x86-
only check in somewhat tidy shape.
Signed-off-by: Jan Beulich <jbeul...@suse.com>
---
Initially I had this (with less code churn) as
#ifdef CONFIG_X86
if ( p2mt == p2m_ram_logdirty )
ret = -EAGAIN;
else
#endif
if ( (p2mt != p2m_ram_rw) ||
!get_page_type(page, PGT_writable_page) )
ret = -EINVAL;
But the "else" placement seemed too ugly to me. Otoh there better
wouldn't be any special casing of log-dirty here (and instead such a
page be converted, perhaps right in check_get_page_from_gfn() when
readonly=false), at which point the odd "else" would go away, and the
if() likely again be preferable over the switch().
--- a/xen/common/argo.c
+++ b/xen/common/argo.c
@@ -1421,15 +1421,24 @@ find_ring_mfn(struct domain *d, gfn_t gf
return ret;
*mfn = page_to_mfn(page);
- if ( !mfn_valid(*mfn) )
- ret = -EINVAL;
+
+ switch ( p2mt )
+ {
+ case p2m_ram_rw:
+ if ( !get_page_and_type(page, d, PGT_writable_page) )
+ ret = -EINVAL;
+ break;
+
#ifdef CONFIG_X86
- else if ( p2mt == p2m_ram_logdirty )
+ case p2m_ram_logdirty:
ret = -EAGAIN;
+ break;
#endif
- else if ( (p2mt != p2m_ram_rw) ||
- !get_page_and_type(page, d, PGT_writable_page) )
+
+ default:
ret = -EINVAL;
+ break;
+ }
put_page(page);
