All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Andi Kleen <ak@linux.intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>,
	"Kuppuswamy,
	Sathyanarayanan"  <sathyanarayanan.kuppuswamy@linux.intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Andy Lutomirski <luto@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Richard Henderson <rth@twiddle.net>,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	James E J Bottomley <James.Bottomley@hansenpartnership.com>,
	Helge Deller <deller@gmx.de>,
	"David S . Miller" <davem@davemloft.net>,
	Arnd Bergmann <arnd@arndb.de>, Jonathan Corbet <corbet@lwn.net>,
	Peter H Anvin <hpa@zytor.com>,
	Dave Hansen <dave.hansen@intel.com>,
	Tony Luck <tony.luck@intel.com>,
	Kirill Shutemov <kirill.shutemov@linux.intel.com>,
	Sean Christopherson <seanjc@google.com>,
	Kuppuswamy Sathyanarayanan <knsathya@kernel.org>,
	X86 ML <x86@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux PCI <linux-pci@vger.kernel.org>,
	linux-alpha@vger.kernel.org, linux-mips@vger.kernel.org,
	linux-parisc@vger.kernel.org, sparclinux@vger.kernel.org,
	linux-arch <linux-arch@vger.kernel.org>,
	Linux Doc Mailing List <linux-doc@vger.kernel.org>,
	virtualization@lists.linux-foundation.org
Subject: Re: [PATCH v4 11/15] pci: Add pci_iomap_shared{,_range}
Date: Fri, 10 Sep 2021 05:54:28 -0400	[thread overview]
Message-ID: <20210910054044-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <69fc30f4-e3e2-add7-ec13-4db3b9cc0cbd@linux.intel.com>

On Mon, Aug 30, 2021 at 05:23:17PM -0700, Andi Kleen wrote:
> 
> On 8/30/2021 1:59 PM, Michael S. Tsirkin wrote:
> > 
> > > Or we can add _audited to the name. ioremap_shared_audited?
> > But it's not the mapping that has to be done in handled special way.
> > It's any data we get from device, not all of it coming from IO, e.g.
> > there's DMA and interrupts that all have to be validated.
> > Wouldn't you say that what is really wanted is just not running
> > unaudited drivers in the first place?
> 
> 
> Yes.

Then ... let's do just that?

> 
> > 
> > > And we've been avoiding that drivers can self declare auditing, we've been
> > > trying to have a separate centralized list so that it's easier to enforce
> > > and avoids any cut'n'paste mistakes.
> > > 
> > > -Andi
> > Now I'm confused. What is proposed here seems to be basically that,
> > drivers need to declare auditing by replacing ioremap with
> > ioremap_shared.
> 
> Auditing is declared on the device model level using a central allow list.

Can we not have an init call allow list instead of, or in addition to, a
device allow list?

> But this cannot do anything to initcalls that run before probe,

Can't we extend module_init so init calls are validated against the
allow list?

> that's why
> an extra level of defense of ioremap opt-in is useful.

OK even assuming this, why is pci_iomap opt-in useful?
That never happens before probe - there's simply no pci_device then.

> But it's not the
> primary mechanism to declare a driver audited, that's the allow list. The
> ioremap is just another mechanism to avoid having to touch a lot of legacy
> drivers.
> 
> If we agree on that then the original proposed semantics of "ioremap_shared"
> may be acceptable?
> 
> -Andi
> 

It looks suspiciously like drivers self-declaring auditing to me which
we both seem to agree is undesirable. What exactly is the difference?

Or are you just trying to disable anything that runs before probe?
In that case I don't see a reason to touch pci drivers though.
These should be fine with just the device model list.

-- 
MST


WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Andi Kleen <ak@linux.intel.com>
Cc: "Kuppuswamy,
	Sathyanarayanan" <sathyanarayanan.kuppuswamy@linux.intel.com>,
	Kuppuswamy Sathyanarayanan <knsathya@kernel.org>,
	Linux Doc Mailing List <linux-doc@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Linux PCI <linux-pci@vger.kernel.org>,
	linux-mips@vger.kernel.org,
	James E J Bottomley <James.Bottomley@hansenpartnership.com>,
	Dave Hansen <dave.hansen@intel.com>,
	Peter H Anvin <hpa@zytor.com>,
	sparclinux@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
	linux-arch <linux-arch@vger.kernel.org>,
	Jonathan Corbet <corbet@lwn.net>, Helge Deller <deller@gmx.de>,
	X86 ML <x86@kernel.org>, Ingo Molnar <mingo@redhat.com>,
	Arnd Bergmann <arnd@arndb.de>, Tony Luck <tony.luck@intel.com>,
	Borislav Petkov <bp@alien8.de>, Andy Lutomirski <luto@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Dan Williams <dan.j.williams@intel.com>,
	virtualization@lists.linux-foundation.org,
	Richard Henderson <rth@twiddle.net>,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	linux-parisc@vger.kernel.org,
	Sean Christopherson <seanjc@google.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-alpha@vger.kernel.org,
	"David S . Miller" <davem@davemloft.net>,
	Kirill Shutemov <kirill.shutemov@linux.intel.com>
Subject: Re: [PATCH v4 11/15] pci: Add pci_iomap_shared{,_range}
Date: Fri, 10 Sep 2021 05:54:28 -0400	[thread overview]
Message-ID: <20210910054044-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <69fc30f4-e3e2-add7-ec13-4db3b9cc0cbd@linux.intel.com>

On Mon, Aug 30, 2021 at 05:23:17PM -0700, Andi Kleen wrote:
> 
> On 8/30/2021 1:59 PM, Michael S. Tsirkin wrote:
> > 
> > > Or we can add _audited to the name. ioremap_shared_audited?
> > But it's not the mapping that has to be done in handled special way.
> > It's any data we get from device, not all of it coming from IO, e.g.
> > there's DMA and interrupts that all have to be validated.
> > Wouldn't you say that what is really wanted is just not running
> > unaudited drivers in the first place?
> 
> 
> Yes.

Then ... let's do just that?

> 
> > 
> > > And we've been avoiding that drivers can self declare auditing, we've been
> > > trying to have a separate centralized list so that it's easier to enforce
> > > and avoids any cut'n'paste mistakes.
> > > 
> > > -Andi
> > Now I'm confused. What is proposed here seems to be basically that,
> > drivers need to declare auditing by replacing ioremap with
> > ioremap_shared.
> 
> Auditing is declared on the device model level using a central allow list.

Can we not have an init call allow list instead of, or in addition to, a
device allow list?

> But this cannot do anything to initcalls that run before probe,

Can't we extend module_init so init calls are validated against the
allow list?

> that's why
> an extra level of defense of ioremap opt-in is useful.

OK even assuming this, why is pci_iomap opt-in useful?
That never happens before probe - there's simply no pci_device then.

> But it's not the
> primary mechanism to declare a driver audited, that's the allow list. The
> ioremap is just another mechanism to avoid having to touch a lot of legacy
> drivers.
> 
> If we agree on that then the original proposed semantics of "ioremap_shared"
> may be acceptable?
> 
> -Andi
> 

It looks suspiciously like drivers self-declaring auditing to me which
we both seem to agree is undesirable. What exactly is the difference?

Or are you just trying to disable anything that runs before probe?
In that case I don't see a reason to touch pci drivers though.
These should be fine with just the device model list.

-- 
MST

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Andi Kleen <ak@linux.intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>,
	"Kuppuswamy,
	Sathyanarayanan" <sathyanarayanan.kuppuswamy@linux.intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Andy Lutomirski <luto@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Richard Henderson <rth@twiddle.net>,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	James E J Bottomley <James.Bottomley@hansenpartnership.com>,
	Helge Deller <deller@gmx.de>,
	"David S . Miller" <davem@davemloft.net>,
	Arnd Bergmann <arnd@arndb.de>, Jonathan Corbet <corbet@lwn.net>,
	Peter H Anvin <hpa@zytor.com>,
	Dave Hansen <dave.hansen@intel.com>,
	Tony Luck <tony.luck@intel.com>,
	Kirill Shutemov <kirill.shutemov@lin>
Subject: Re: [PATCH v4 11/15] pci: Add pci_iomap_shared{,_range}
Date: Fri, 10 Sep 2021 05:54:28 -0400	[thread overview]
Message-ID: <20210910054044-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <69fc30f4-e3e2-add7-ec13-4db3b9cc0cbd@linux.intel.com>

On Mon, Aug 30, 2021 at 05:23:17PM -0700, Andi Kleen wrote:
> 
> On 8/30/2021 1:59 PM, Michael S. Tsirkin wrote:
> > 
> > > Or we can add _audited to the name. ioremap_shared_audited?
> > But it's not the mapping that has to be done in handled special way.
> > It's any data we get from device, not all of it coming from IO, e.g.
> > there's DMA and interrupts that all have to be validated.
> > Wouldn't you say that what is really wanted is just not running
> > unaudited drivers in the first place?
> 
> 
> Yes.

Then ... let's do just that?

> 
> > 
> > > And we've been avoiding that drivers can self declare auditing, we've been
> > > trying to have a separate centralized list so that it's easier to enforce
> > > and avoids any cut'n'paste mistakes.
> > > 
> > > -Andi
> > Now I'm confused. What is proposed here seems to be basically that,
> > drivers need to declare auditing by replacing ioremap with
> > ioremap_shared.
> 
> Auditing is declared on the device model level using a central allow list.

Can we not have an init call allow list instead of, or in addition to, a
device allow list?

> But this cannot do anything to initcalls that run before probe,

Can't we extend module_init so init calls are validated against the
allow list?

> that's why
> an extra level of defense of ioremap opt-in is useful.

OK even assuming this, why is pci_iomap opt-in useful?
That never happens before probe - there's simply no pci_device then.

> But it's not the
> primary mechanism to declare a driver audited, that's the allow list. The
> ioremap is just another mechanism to avoid having to touch a lot of legacy
> drivers.
> 
> If we agree on that then the original proposed semantics of "ioremap_shared"
> may be acceptable?
> 
> -Andi
> 

It looks suspiciously like drivers self-declaring auditing to me which
we both seem to agree is undesirable. What exactly is the difference?

Or are you just trying to disable anything that runs before probe?
In that case I don't see a reason to touch pci drivers though.
These should be fine with just the device model list.

-- 
MST


  reply	other threads:[~2021-09-10  9:54 UTC|newest]

Thread overview: 129+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-05  0:52 [PATCH v4 00/15] Add TDX Guest Support (shared-mm support) Kuppuswamy Sathyanarayanan
2021-08-05  0:52 ` [PATCH v4 01/15] x86/mm: Move force_dma_unencrypted() to common code Kuppuswamy Sathyanarayanan
2021-08-05  0:52 ` [PATCH v4 02/15] x86/tdx: Exclude Shared bit from physical_mask Kuppuswamy Sathyanarayanan
2021-08-05  0:52 ` [PATCH v4 03/15] x86/tdx: Make pages shared in ioremap() Kuppuswamy Sathyanarayanan
2021-08-05  0:52 ` [PATCH v4 04/15] x86/tdx: Add helper to do MapGPA hypercall Kuppuswamy Sathyanarayanan
2021-08-05  0:52 ` [PATCH v4 05/15] x86/tdx: Make DMA pages shared Kuppuswamy Sathyanarayanan
2021-08-05  0:52 ` [PATCH v4 06/15] x86/kvm: Use bounce buffers for TD guest Kuppuswamy Sathyanarayanan
2021-08-05  0:52 ` [PATCH v4 07/15] x86/tdx: ioapic: Add shared bit for IOAPIC base address Kuppuswamy Sathyanarayanan
2021-08-05  0:52 ` [PATCH v4 08/15] x86/tdx: Enable shared memory protected guest flags for TDX guest Kuppuswamy Sathyanarayanan
2021-08-05  0:52 ` [PATCH v4 09/15] pci: Consolidate pci_iomap* and pci_iomap*wc Kuppuswamy Sathyanarayanan
2021-08-12 19:43   ` Bjorn Helgaas
2021-08-12 19:43     ` Bjorn Helgaas
2021-08-12 19:43     ` Bjorn Helgaas
2021-08-12 22:11     ` Andi Kleen
2021-08-12 22:11       ` Andi Kleen
2021-08-12 22:11       ` Andi Kleen
2021-08-12 22:29     ` Kuppuswamy, Sathyanarayanan
2021-08-12 22:29       ` Kuppuswamy, Sathyanarayanan
2021-08-05  0:52 ` [PATCH v4 10/15] asm/io.h: Add ioremap_shared fallback Kuppuswamy Sathyanarayanan
2021-08-12 19:46   ` Bjorn Helgaas
2021-08-12 19:46     ` Bjorn Helgaas
2021-08-12 19:46     ` Bjorn Helgaas
2021-08-13  7:58   ` Christoph Hellwig
2021-08-13  7:58     ` Christoph Hellwig
2021-08-13  7:58     ` Christoph Hellwig
2021-08-05  0:52 ` [PATCH v4 11/15] pci: Add pci_iomap_shared{,_range} Kuppuswamy Sathyanarayanan
2021-08-13  8:02   ` Christoph Hellwig
2021-08-13  8:02     ` Christoph Hellwig
2021-08-13  8:02     ` Christoph Hellwig
2021-08-23 23:56   ` Michael S. Tsirkin
2021-08-23 23:56     ` Michael S. Tsirkin
2021-08-23 23:56     ` Michael S. Tsirkin
2021-08-24  0:30     ` Kuppuswamy, Sathyanarayanan
2021-08-24  0:30       ` Kuppuswamy, Sathyanarayanan
2021-08-24  1:04       ` Dan Williams
2021-08-24  1:04         ` Dan Williams
2021-08-24  1:04         ` Dan Williams
2021-08-24  2:14         ` Andi Kleen
2021-08-24  2:14           ` Andi Kleen
2021-08-24  2:14           ` Andi Kleen
2021-08-24  9:47           ` Michael S. Tsirkin
2021-08-24  9:47             ` Michael S. Tsirkin
2021-08-24  9:47             ` Michael S. Tsirkin
2021-08-24 17:20             ` Andi Kleen
2021-08-24 17:20               ` Andi Kleen
2021-08-24 17:20               ` Andi Kleen
2021-08-24 18:55               ` Bjorn Helgaas
2021-08-24 18:55                 ` Bjorn Helgaas
2021-08-24 18:55                 ` Bjorn Helgaas
2021-08-24 20:14                 ` Andi Kleen
2021-08-24 20:14                   ` Andi Kleen
2021-08-24 20:14                   ` Andi Kleen
2021-08-24 20:31                   ` Bjorn Helgaas
2021-08-24 20:31                     ` Bjorn Helgaas
2021-08-24 20:31                     ` Bjorn Helgaas
2021-08-24 20:50                     ` Andi Kleen
2021-08-24 20:50                       ` Andi Kleen
2021-08-24 20:50                       ` Andi Kleen
2021-08-24 21:05                       ` Dan Williams
2021-08-24 21:05                         ` Dan Williams
2021-08-24 21:05                         ` Dan Williams
2021-08-25 14:52                       ` Bjorn Helgaas
2021-08-25 14:52                         ` Bjorn Helgaas
2021-08-25 14:52                         ` Bjorn Helgaas
2021-08-24 21:55                 ` Rajat Jain
2021-08-24 21:55                   ` Rajat Jain
2021-08-29 15:27               ` Michael S. Tsirkin
2021-08-29 15:27                 ` Michael S. Tsirkin
2021-08-29 15:27                 ` Michael S. Tsirkin
2021-08-29 16:17                 ` Andi Kleen
2021-08-29 16:17                   ` Andi Kleen
2021-08-29 16:17                   ` Andi Kleen
2021-08-29 22:26                   ` Michael S. Tsirkin
2021-08-29 22:26                     ` Michael S. Tsirkin
2021-08-29 22:26                     ` Michael S. Tsirkin
2021-08-30  5:11                     ` Andi Kleen
2021-08-30  5:11                       ` Andi Kleen
2021-08-30  5:11                       ` Andi Kleen
2021-08-30 20:59                       ` Michael S. Tsirkin
2021-08-30 20:59                         ` Michael S. Tsirkin
2021-08-30 20:59                         ` Michael S. Tsirkin
2021-08-31  0:23                         ` Andi Kleen
2021-08-31  0:23                           ` Andi Kleen
2021-08-31  0:23                           ` Andi Kleen
2021-09-10  9:54                           ` Michael S. Tsirkin [this message]
2021-09-10  9:54                             ` Michael S. Tsirkin
2021-09-10  9:54                             ` Michael S. Tsirkin
2021-09-10 16:34                             ` Andi Kleen
2021-09-10 16:34                               ` Andi Kleen
2021-09-10 16:34                               ` Andi Kleen
2021-09-11 23:54                               ` Michael S. Tsirkin
2021-09-11 23:54                                 ` Michael S. Tsirkin
2021-09-11 23:54                                 ` Michael S. Tsirkin
2021-09-13  5:53                                 ` Michael S. Tsirkin
2021-09-13  5:53                                   ` Michael S. Tsirkin
2021-09-13  5:53                                   ` Michael S. Tsirkin
2021-09-24 22:43                                 ` Andi Kleen
2021-09-24 22:43                                   ` Andi Kleen
2021-09-24 22:43                                   ` Andi Kleen
2021-09-27  9:07                                   ` Michael S. Tsirkin
2021-09-27  9:07                                     ` Michael S. Tsirkin
2021-09-27  9:07                                     ` Michael S. Tsirkin
2021-08-24 21:56         ` Rajat Jain
2021-08-24 21:56           ` Rajat Jain
2021-08-24 21:59           ` Dan Williams
2021-08-24 21:59             ` Dan Williams
2021-08-24 21:59             ` Dan Williams
2021-08-24  7:07       ` Christoph Hellwig
2021-08-24  7:07         ` Christoph Hellwig
2021-08-24  7:07         ` Christoph Hellwig
2021-08-24 17:04         ` Andi Kleen
2021-08-24 17:04           ` Andi Kleen
2021-08-24 17:04           ` Andi Kleen
2021-08-29 15:34           ` Michael S. Tsirkin
2021-08-29 15:34             ` Michael S. Tsirkin
2021-08-29 15:34             ` Michael S. Tsirkin
2021-08-29 16:43             ` Andi Kleen
2021-08-29 16:43               ` Andi Kleen
2021-08-29 16:43               ` Andi Kleen
2021-08-24  9:12       ` Michael S. Tsirkin
2021-08-24  9:12         ` Michael S. Tsirkin
2021-08-24  9:12         ` Michael S. Tsirkin
2021-08-05  0:52 ` [PATCH v4 12/15] pci: Mark MSI data shared Kuppuswamy Sathyanarayanan
2021-08-13  8:07   ` Christoph Hellwig
2021-08-13  8:07     ` Christoph Hellwig
2021-08-13  8:07     ` Christoph Hellwig
2021-08-05  0:52 ` [PATCH v4 13/15] virtio: Use shared mappings for virtio PCI devices Kuppuswamy Sathyanarayanan
2021-08-05  0:52 ` [PATCH v4 14/15] x86/tdx: Implement ioremap_shared for x86 Kuppuswamy Sathyanarayanan
2021-08-05  0:52 ` [PATCH v4 15/15] x86/tdx: Add cmdline option to force use of ioremap_shared Kuppuswamy Sathyanarayanan

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=20210910054044-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=ak@linux.intel.com \
    --cc=arnd@arndb.de \
    --cc=bhelgaas@google.com \
    --cc=bp@alien8.de \
    --cc=corbet@lwn.net \
    --cc=dan.j.williams@intel.com \
    --cc=dave.hansen@intel.com \
    --cc=davem@davemloft.net \
    --cc=deller@gmx.de \
    --cc=hpa@zytor.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=knsathya@kernel.org \
    --cc=linux-alpha@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=linux-parisc@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rth@twiddle.net \
    --cc=sathyanarayanan.kuppuswamy@linux.intel.com \
    --cc=seanjc@google.com \
    --cc=sparclinux@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=tsbogend@alpha.franken.de \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=x86@kernel.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.