All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andi Kleen <ak@linux.intel.com>
To: "Michael S. Tsirkin" <mst@redhat.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: Sun, 29 Aug 2021 22:11:46 -0700	[thread overview]
Message-ID: <3a88a255-a528-b00a-912b-e71198d5f58f@linux.intel.com> (raw)
In-Reply-To: <20210829181635-mutt-send-email-mst@kernel.org>


On 8/29/2021 3:26 PM, Michael S. Tsirkin wrote:
> On Sun, Aug 29, 2021 at 09:17:53AM -0700, Andi Kleen wrote:
>> Also I changing this single call really that bad? It's not that we changing
>> anything drastic here, just give the low level subsystem a better hint about
>> the intention. If you don't like the function name, could make it an
>> argument instead?
> My point however is that the API should say that the
> driver has been audited,

We have that status in the struct device. If you want to tie the ioremap 
to that we could define a ioremap_device() with a device argument and 
decide based on that.

Or we can add _audited to the name. ioremap_shared_audited?

> not that the mapping has been
> done in some special way. For example the mapping can be
> in some kind of wrapper, not directly in the driver.
> However you want the driver validated, not the wrapper.
>
> Here's an idea:


I don't think magic differences of API behavior based on some define are 
a good idea.  That's easy to miss.

That's a "COME FROM" in API design.

Also it wouldn't handle the case that a driver has both private and 
shared ioremaps, e.g. for BIOS structures.

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


WARNING: multiple messages have this Message-ID (diff)
From: Andi Kleen <ak@linux.intel.com>
To: "Michael S. Tsirkin" <mst@redhat.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: Sun, 29 Aug 2021 22:11:46 -0700	[thread overview]
Message-ID: <3a88a255-a528-b00a-912b-e71198d5f58f@linux.intel.com> (raw)
In-Reply-To: <20210829181635-mutt-send-email-mst@kernel.org>


On 8/29/2021 3:26 PM, Michael S. Tsirkin wrote:
> On Sun, Aug 29, 2021 at 09:17:53AM -0700, Andi Kleen wrote:
>> Also I changing this single call really that bad? It's not that we changing
>> anything drastic here, just give the low level subsystem a better hint about
>> the intention. If you don't like the function name, could make it an
>> argument instead?
> My point however is that the API should say that the
> driver has been audited,

We have that status in the struct device. If you want to tie the ioremap 
to that we could define a ioremap_device() with a device argument and 
decide based on that.

Or we can add _audited to the name. ioremap_shared_audited?

> not that the mapping has been
> done in some special way. For example the mapping can be
> in some kind of wrapper, not directly in the driver.
> However you want the driver validated, not the wrapper.
>
> Here's an idea:


I don't think magic differences of API behavior based on some define are 
a good idea.  That's easy to miss.

That's a "COME FROM" in API design.

Also it wouldn't handle the case that a driver has both private and 
shared ioremaps, e.g. for BIOS structures.

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


WARNING: multiple messages have this Message-ID (diff)
From: Andi Kleen <ak@linux.intel.com>
To: "Michael S. Tsirkin" <mst@redhat.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: Sun, 29 Aug 2021 22:11:46 -0700	[thread overview]
Message-ID: <3a88a255-a528-b00a-912b-e71198d5f58f@linux.intel.com> (raw)
In-Reply-To: <20210829181635-mutt-send-email-mst@kernel.org>


On 8/29/2021 3:26 PM, Michael S. Tsirkin wrote:
> On Sun, Aug 29, 2021 at 09:17:53AM -0700, Andi Kleen wrote:
>> Also I changing this single call really that bad? It's not that we changing
>> anything drastic here, just give the low level subsystem a better hint about
>> the intention. If you don't like the function name, could make it an
>> argument instead?
> My point however is that the API should say that the
> driver has been audited,

We have that status in the struct device. If you want to tie the ioremap 
to that we could define a ioremap_device() with a device argument and 
decide based on that.

Or we can add _audited to the name. ioremap_shared_audited?

> not that the mapping has been
> done in some special way. For example the mapping can be
> in some kind of wrapper, not directly in the driver.
> However you want the driver validated, not the wrapper.
>
> Here's an idea:


I don't think magic differences of API behavior based on some define are 
a good idea.  That's easy to miss.

That's a "COME FROM" in API design.

Also it wouldn't handle the case that a driver has both private and 
shared ioremaps, e.g. for BIOS structures.

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

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

  reply	other threads:[~2021-08-30  5:11 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 [this message]
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
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=3a88a255-a528-b00a-912b-e71198d5f58f@linux.intel.com \
    --to=ak@linux.intel.com \
    --cc=James.Bottomley@hansenpartnership.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@lin \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=mst@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rth@twiddle.net \
    --cc=sathyanarayanan.kuppuswamy@linux.intel.com \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=tsbogend@alpha.franken.de \
    /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.