On 8/8/2016 11:40 PM, Jan Beulich wrote:
On 12.07.16 at 11:02, <yu.c.zh...@linux.intel.com> wrote:
@@ -178,8 +179,34 @@ static int hvmemul_do_io(
break;
case X86EMUL_UNHANDLEABLE:
{
- struct hvm_ioreq_server *s =
- hvm_select_ioreq_server(curr->domain, &p);
+ struct hvm_ioreq_server *s;
+
+ if ( is_mmio )
+ {
+ unsigned long gmfn = paddr_to_pfn(addr);
+ p2m_type_t p2mt;
+
+ (void) get_gfn_query_unlocked(currd, gmfn, &p2mt);
+
+ if ( p2mt == p2m_ioreq_server )
+ {
+ unsigned int flags;
+
+ if ( dir != IOREQ_WRITE )
+ s = NULL;
+ else
+ {
+ s = p2m_get_ioreq_server(currd, &flags);
+
+ if ( !(flags & P2M_IOREQ_HANDLE_WRITE_ACCESS) )
+ s = NULL;
+ }
+ }
+ else
+ s = hvm_select_ioreq_server(currd, &p);
+ }
+ else
+ s = hvm_select_ioreq_server(currd, &p);
Wouldn't it both be more natural and make the logic even easier
to follow if s got set to NULL up front, all the "else"-s dropped,
and a simple
if ( !s )
s = hvm_select_ioreq_server(currd, &p);
be done in the end?
Sorry, Jan. I tried to simplify above code, but found the new code is
still not very
clean, because in some cases the s is supposed to return NULL instead
of to be
set from the hvm_select_ioreq_server().
To keep the same logic, the simplified code looks like this:
case X86EMUL_UNHANDLEABLE:
{
- struct hvm_ioreq_server *s =
- hvm_select_ioreq_server(curr->domain, &p);
+ struct hvm_ioreq_server *s = NULL;
+ p2m_type_t p2mt = p2m_invalid;
+
+ if ( is_mmio && dir == IOREQ_WRITE )
+ {
+ unsigned long gmfn = paddr_to_pfn(addr);
+
+ (void) get_gfn_query_unlocked(currd, gmfn, &p2mt);
+
+ if ( p2mt == p2m_ioreq_server )
+ {
+ unsigned int flags;
+
+ s = p2m_get_ioreq_server(currd, &flags);
+ if ( !(flags & XEN_HVMOP_IOREQ_MEM_ACCESS_WRITE) )
+ s = NULL;
+ }
+ }
+
+ if ( !s && p2mt != p2m_ioreq_server )
+ s = hvm_select_ioreq_server(currd, &p);
/* If there is no suitable backing DM, just ignore accesses */
if ( !s )
As you can see, definition of p2mt is moved outside the if ( is_mmio )
judgement,
and is checked against p2m_ioreq_server before we search the ioreq
server's rangeset
in hvm_select_ioreq_server(). So I am not quite satisfied with this
simplification.
Any suggestions?
[snip]
Yu
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel