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 Thu, May 05, 2016 at 08:57:40AM +0200, Alexander Gordeev wrote:
> On Wed, May 04, 2016 at 04:12:33PM +0200, Andrew Jones wrote:
> > > 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.
> > 
> > You don't really need to write vmap_uncached unless you plan to move
> > unit tests to it too. We'd need sign-off from the unit test developers
> > for that though. BTW, I'd still add a vmap_uncached, but it can
> > certainly wrap a vmap_prot. I wouldn't extend vmap though. The default
> > flags will almost always be correct, so having to pass them in (or a
> > zero to be ORed with them) all the time wouldn't be nice.
> 
> I re-read it few times and still do not get if PCD for ioremap() is
> desirable or not :) So if yes, I would go this way:

I think it is, but you'll need sign-off from the unit test developers
for any unit test you modify to use it. If you choose not to move
any unit tests to it, then there's really no point in writing it.

> 
> 	vmap_prot( ..., prot )
> 	{
> 		... = <default prot> | prot

I wouldn't OR in the default prot here (except maybe PTE_PRESENT).
Otherwise there's no point in creating this function, as it won't
be flexible enough for all use cases.

> 	}
> 
> 	#define vmap( ... )	vmap_prot( ..., 0 )
> 
> And if vmap_uncached is still needed (not sure why would?) - as you said:

Because here you OR in the defaults, plus PCD, making it easy for
users to get uncached mappings.

Basically, vmap_prot is the super flexible API, users have full
control, but it's not super user friendly for users that don't
need full control. Everything else can wrap around it passing in
defaults plus/minus tweaks to those defaults.

> 
> 	#define vmap_uncached( ... )	vmap_prot( ..., <PCD prot> )
> 
> Then turn x86 ioremap() to the PCD variant. And all of these with a follow-up
> series.
> 
> > drew
> --
> 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