Re: [kvm-unit-tests PATCH v6 11/11] x86: io: Factor out ioremap()

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

 



On Wed, May 04, 2016 at 02:24:04PM +0200, Andrew Jones wrote:
> On Wed, May 04, 2016 at 12:31:36PM +0200, Alexander Gordeev wrote:
> > Cc: Andrew Jones <drjones@xxxxxxxxxx>
> > Cc: Thomas Huth <thuth@xxxxxxxxxx>
> > Cc: Radim Krčmář <rkrcmar@xxxxxxxxxx>
> > Reviewed-by: Andrew Jones <drjones@xxxxxxxxxx>
> > Signed-off-by: Alexander Gordeev <agordeev@xxxxxxxxxx>
> > ---
> >  lib/x86/asm/io.h |  2 ++
> >  lib/x86/io.c     | 15 +++++++++++++++
> >  x86/vmexit.c     | 10 ++--------
> >  3 files changed, 19 insertions(+), 8 deletions(-)
> > 
> > diff --git a/lib/x86/asm/io.h b/lib/x86/asm/io.h
> > index 83387b5..2436822 100644
> > --- a/lib/x86/asm/io.h
> > +++ b/lib/x86/asm/io.h
> > @@ -49,4 +49,6 @@ static inline void *phys_to_virt(unsigned long phys)
> >      return (void *)phys;
> >  }
> >  
> > +void __iomem *ioremap(phys_addr_t phys_addr, size_t size);
> > +
> >  #endif
> > diff --git a/lib/x86/io.c b/lib/x86/io.c
> > index d396d42..c49926a 100644
> > --- a/lib/x86/io.c
> > +++ b/lib/x86/io.c
> > @@ -1,6 +1,8 @@
> >  #include "libcflat.h"
> > +#include "vm.h"
> >  #include "smp.h"
> >  #include "asm/io.h"
> > +#include "asm/page.h"
> >  #ifndef USE_SERIAL
> >  #define USE_SERIAL
> >  #endif
> > @@ -81,3 +83,16 @@ void exit(int code)
> >          asm volatile("out %0, %1" : : "a"(code), "d"((short)0xf4));
> >  #endif
> >  }
> > +
> > +void __iomem *ioremap(phys_addr_t phys_addr, size_t size)
> > +{
> > +	phys_addr_t base = phys_addr & PAGE_MASK;
> > +	phys_addr_t offset = phys_addr - base;
> > +
> > +	/*
> > +	 * The kernel sets PTEs for an ioremap() with page cache disabled.
> > +	 * It would make sense that I/O mappings would be uncached -
> > +	 * and may help us find bugs when we properly map that way.
> 
> You left out the 'but' ("...that way, but we don't currently. Maybe we
> should add a vmap_uncached?"), or something like that.

If this one is better?

"
Andrew Jones noted: "The kernel sets the PTEs for an ioremap with PCD,
page cache disabled. I guess we can live without that, but it would
make sense that I/O mappings would be uncached - and may help us find
bugs when we properly map that way."
"

As of vmap_uncached - I guess since vmap() has a broader "scope" than
just IO it would make sense to introduce vmap_prot() instead. Or
add an extra page protection parameter to the existing vmap(). I could
do that with a follow-up patch.

> > +	 */
> > +	return vmap(phys_addr, size) + offset;
> > +}
> > diff --git a/x86/vmexit.c b/x86/vmexit.c
> > index db7dbd8..c2e1e49 100644
> > --- a/x86/vmexit.c
> > +++ b/x86/vmexit.c
> > @@ -371,8 +371,7 @@ int main(int ac, char **av)
> >  {
> >  	struct fadt_descriptor_rev1 *fadt;
> >  	int i;
> > -	unsigned long membar = 0, base, offset;
> > -	void *m;
> > +	unsigned long membar = 0;
> >  	pcidevaddr_t pcidev;
> >  
> >  	smp_init();
> > @@ -394,12 +393,7 @@ int main(int ac, char **av)
> >  			}
> >  			if (pci_bar_is_memory(pcidev, i)) {
> >  				membar = pci_bar_addr(pcidev, i);
> > -				base = membar & ~4095;
> > -				offset = membar - base;
> > -				m = alloc_vpages(1);
> > -				
> > -				install_page((void *)read_cr3(), base, m);
> > -				pci_test.memaddr = m + offset;
> > +				pci_test.memaddr = ioremap(membar, PAGE_SIZE);
> >  			} else {
> >  				pci_test.iobar = pci_bar_addr(pcidev, i);
> >  			}
> > -- 
> > 1.8.3.1
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux