[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v7] x86/emulate: Send vm_event from emulate



On 20.08.2019 22:11, Andrew Cooper wrote:
On 30/07/2019 15:54, Jan Beulich wrote:
@@ -622,14 +622,22 @@ static void *hvmemul_map_linear_addr(
                }

                if ( p2mt == p2m_ioreq_server )
-            {
-                err = NULL;
                    goto out;
-            }

                ASSERT(p2mt == p2m_ram_logdirty || !p2m_is_readonly(p2mt));
+
+            if ( curr->arch.vm_event &&
+                 curr->arch.vm_event->send_event &&
+                 hvm_emulate_send_vm_event(addr, gfn, pfec) )
+                err = ERR_PTR(~X86EMUL_RETRY);
            }
        }
+    /* Check if any vm_event was sent */
+    if ( err )
+        goto out;

        /* Entire access within a single frame? */
        if ( nr_frames == 1 )
First of all I have to apologize: In earlier replies I referred
to update_map_err(). I notice only now that this is a still
pending change of mine, which Andrew continues to object to,
while I continue to think it (in one form or another) is needed:
https://lists.xenproject.org/archives/html/xen-devel/2018-09/msg01250.html

Given the unpatched code, I think your change is correct, but
quite possibly your earlier variant was, too. But since the
unpatched code is imo wrong, I'd prefer if the VM event side
change was put on top of the fixed code, in order to not further
complicate the actual fix (which we may also want to backport).

Andrew, as to that old pending patch, I'm afraid I haven't been
convinced in the slightest by your argumentation, regardless of
the actual behavior of the XTF test you've created.

So what?  You want your change taken anyway despite evidence that it is
wrong?

  There are
two fundamental points you've not addressed during the earlier
discussion:
1) For a guest behavior should be entirely transparent as far as
2nd level translation goes, unless the _only_ issue results from
it. That's because on bare hardware there simply is no 2nd level
translation.
2) Somewhat related, consider the case of the guest handling the
#PF on the second half of the access by a means which makes the
reason for the 2nd stage "fault" go away, or not recur. In that
case we've wrongly (i.e. at least needlessly) dealt with the 2nd
stage "fault".

For both of these, do you actually have an example where you believe
Xen's logic currently goes wrong?  All I see, looking though the
threads, is unsubstantiated claims that the current behaviour is wrong.

Hmm, I thought we're both still recalling the case this started from:
ballooned-out page handling kicking in when the guest expects a page
fault (based on its own page tables).

I am, btw, not convinced that the behavior as you've observed it
is actually "correct" in the sense of "sensible".

You're entitled to the believe that this isn't sensible if you wish.

It doesn't make it relevant to the argument.  Relevant arguments would
be identifying, a bug in my XTF test, or counterexample where the CPUs
do an opposite thing, or a passage in a spec which make a statement
supporting your claim.

As far as I am concerned, it is perfectly sensible and logical
behaviour.  To complete an instruction which straddles a page boundary,
it is necessary to have both translations available in the TLB, which
requires two EPT-walks to have already completed correctly.

SDM Vol 3 28.2.3.3 is very clear on the matter.  All translations to the
ultimate physical addresses get established first (I.e. the TLB fills
complete) before any access rights get considered.  This means that
ordering of #PF and EPT misconfig/violation is complicated by their dual
nature for failures.

In reality, I think the current code in Xen will get the priority of
second and first stage access right fault inverted, but its a damn sight
closer to how the CPU behaves than the proposed patch, which would get
first staged access rights mixed up with second stage translation faults.

I consider your position as perfectly valid to take. It's just that, as
in so many other cases, it's not the only valid one (imo). You judge
from observed behavior, which is fine. You don't, however, address my
argument of there not being 2nd stage translation at all from guest
pov: The change made results in the expected behavior if there was no
2nd stage translation. And it is my view of virtualization that the
goal should be to provide guest visible behavior matching the
unvirtualized case as much as possible.

Jan

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.