All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jan Beulich" <JBeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Juergen Gross <jgross@suse.com>,
	Igor Druzhinin <igor.druzhinin@citrix.com>,
	Kevin Tian <kevin.tian@intel.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Tim Deegan <tim@xen.org>, Lars Kurth <lars.kurth@xenproject.org>,
	Xen-devel <xen-devel@lists.xen.org>,
	Julien Grall <julien.grall@arm.com>,
	Paul Durrant <paul.durrant@citrix.com>,
	Jun Nakajima <jun.nakajima@intel.com>,
	Ian Jackson <ian.jackson@citrix.com>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [PATCH for-4.12] passthrough/vtd: Drop the "workaround_bios_bug" logic entirely
Date: Wed, 27 Mar 2019 09:41:57 -0600	[thread overview]
Message-ID: <5C9B99C502000078002222D2@prv1-mh.provo.novell.com> (raw)
In-Reply-To: <0fd617eb-52cd-914f-aff5-3c2de35b081e@citrix.com>

>>> On 27.03.19 at 15:38, <andrew.cooper3@citrix.com> wrote:
> There is absolutely nothing *at all* which guarantees that just because
> a number of devices are identified to be behind specific IOMMUs, that
> DMA wont start appearing from elsewhere in the system.

I fully agree here.

> Security of the system when it comes to IOMMUs *is and always will be* a
> mutually cooperative and trusting relationship between Xen and the firmware.
> 
> The notion of "I'm safe because there were no inconsistencies in a piece
> of information I have to trust fully" is security theatre, not security.

Doing our best to sanity check information we're handed is, I think,
not just "security theater".

>>> Disabling the IOMMU prevents the system from booting with a PVH dom0.
>> But doing what you did is not the only way of getting around this.
>> Defaulting to workaround_bios_bug=1 in the PVH case would be
>> another, as would be a mode in which the IOMMU exists for Dom0's
>> purposes only (i.e. still disallowing any pass-through to DomU-s).
> 
> A discussion along these lines might be appropriate in the middle of a
> dev cycle, and might even be valid for a discussion of future improvements.
> 
> It is not appropriate for resolving an issue identified as a 4.12
> blocker by the RM, on a timescale which needs to fit into the 4.12
> release plans.

Okay, I clearly must have missed the mail where this was flagged
as release critical.

Irrespective of this I don't think, though, that a pending release
is sufficient justification to rush in a controversial patch. We're
yet to hear from Kevin, but as you can see I would not have ack-
ed the patch. Emergency ack-s from The Rest maintainers ought
to be given on the assumption that the actual maintainer(s)
would likely not object. I don't think George tried to violate this,
i.e. I think he was assuming that the maintainer(s) would agree,
but as we see this was wrong in this case.

> And if you'd not broken the behaviour back in 4.2, this class of system
> would have been discovered the first time someone tried booting Xen on
> it, not at the point someone is trying to upgrade 4.11 to 4.12.

Correct, and I take the blame for this, but I don't think it helps
the situation. If the problem had been discovered earlier, I
don't think the fix would have been to rip out that code
altogether.

>>> I have made a statement, backed up with specific reference to the code
>>> which, to the best of my ability, demonstrates it to be true.
>>>
>>> If you believe contrary then clearly identify the fault in my reasoning.
>> I did, by pointing out the earlier regression, which you elected to
>> ignore altogether in your reply.
> 
> You identified why Xen 4.11 didn't behave in the way you expected.
> 
> In doing so, you also demonstrated why the commit message was, in fact,
> correct.

Well, we continue to disagree here. It was at best misleading and/or
incomplete.

> Like other parts of this thread, it was deviating sufficiently
> off-topic/relevance that I chose to trim it.  I will continue doing so
> in an effort to reduce the amount of my time that this thread is
> wasting.  I don't know for certain, but I expect you've also got better
> things to do with your time than arguing over off-topic aspects of this
> thread.

Purely from a technical pov the discussion on whether this should
have been rushed in may indeed be considered off topic. But this
doesn't means it's irrelevant. Would you have liked it better if I
had started a separate thread, e.g. by formally requesting a
revert?

I can understand your frustration, but it's not you alone who is
frustrated. Throughout this discussion I've not seen a single
sign that you would be willing to find a compromise. I'm sorry to
repeat myself, but it imo is not reasonable to assume that
your way of thinking is the only possible or reasonable one.
Claiming that anything else is beyond "common sense" is, well,
offending.

And yes, I do have better things to do with my time. But I don't
think I can leave uncommented how things have gone here, if
nothing else then in the hopes that it wouldn't repeat again.

Jan


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

  reply	other threads:[~2019-03-27 15:41 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-21 20:26 [PATCH for-4.12] passthrough/vtd: Drop the "workaround_bios_bug" logic entirely Andrew Cooper
2019-03-21 20:47 ` Igor Druzhinin
2019-03-22  1:43 ` Roger Pau Monné
2019-03-22  6:07 ` Juergen Gross
2019-03-22 11:53 ` George Dunlap
2019-03-25 15:24 ` Jan Beulich
2019-03-25 17:36   ` Andrew Cooper
2019-03-26  9:08     ` Jan Beulich
2019-03-26 12:43       ` Andrew Cooper
2019-03-26 13:39         ` Jan Beulich
2019-03-27 14:38           ` Andrew Cooper
2019-03-27 15:41             ` Jan Beulich [this message]
2019-03-28 14:49             ` George Dunlap
2019-03-28 15:06           ` George Dunlap
2019-03-28 16:12             ` Jan Beulich
2019-03-27 11:02   ` George Dunlap
2019-03-27 11:46     ` Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5C9B99C502000078002222D2@prv1-mh.provo.novell.com \
    --to=jbeulich@suse.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=ian.jackson@citrix.com \
    --cc=igor.druzhinin@citrix.com \
    --cc=jgross@suse.com \
    --cc=julien.grall@arm.com \
    --cc=jun.nakajima@intel.com \
    --cc=kevin.tian@intel.com \
    --cc=konrad.wilk@oracle.com \
    --cc=lars.kurth@xenproject.org \
    --cc=paul.durrant@citrix.com \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=tim@xen.org \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.