Re: [Xen-devel] [PATCH 0/5] pciback: new configuration space fields, permissive flag

[Ryan <hap9@xxxxxxxxxxxxxx>]

On Thu, 2006-04-13 at 10:06 +0100, Keir Fraser wrote:
> 
> It seems to me that this splits policy decisions on permissiveness of 
> access to a particular PCI device between user space and kernel. 
> Specifically, to make good automatic use of the per-device permissive 
> flag we will need a mapping in user space from device ids to correct 
> setting of the flag. If you leave it manually to the user, you know 
> which way it will always be set. :-) At the same time, in the kernel we 
> have a mapping from device ids to filtering rules, which are just 
> another facet of filtering policy.
> 
> I think it would be much neater to implement only the enforcement 
> mechanisms in the kernel driver, and to move all the rules about which 
> registers may be accessed for which device types out into user space. 
> Then, when binding a PCI device to pciback we would also squirt the 
> filtering rules into the kernel. This seems to me preferable for a 
> number of reasons:
>   1. We don't end up with a scaling mess of extra C source files for 
> every new device we come across.
>   2. Maintain the rules in an easier to edit format in text files
>   3. One place we maintain mappings from device ids -> filtering policies
> 
> The main downside is the extra work to push the rules into the kernel 
> and do whatever parsing is required. It does feel like the right way to 
> go, though.
> 

I'd thought about this option and perhaps it is a good option for
handling device specific configuration fields. Another downside is that
you lose the ability to do any custom handling of those fields (at least
not without a lot of complicated back-and-forth between the kernel and
some user-space pci daemon). For example, I don't think it would be
possible to do in user-space what we do now for intercepting calls to
registers like PCI_COMMAND or the new PCI power management stuff I added
with these patches (which make calls to core pci functions in the
kernel). All a user-space policy could then contain is whether a given
field is writable or not (and maybe a mask of exactly which bits are
writable). However, that's probably enough for most devices (although I
can't say for certain having only had a cursory look at the tg3 driver
and no others). I think the header fields and structures on the
capability list will need to remain in kernel-space.

I also wasn't sure when was a good time to inject these rules into the
pciback driver. Right now, the configuration fields get added to a card
when it is probed. I suppose I could generate some kind of hotplug event
to trigger that. Or perhaps some kind of lazy initialization that takes
place through xend the first time that a PCI device is given to a driver
domain.

I see your point about the permissive flag. While I think this method is
better than the global flag, I think you're right: we can improve it
more. I could also do a lazy initialization on the permissive flag
(perhaps moving it from a sysfs attribute to xend matching a device id
and setting it in XenStore). Is that more along the lines of what you
were thinking?

Do you have any other comments/concerns about adding the capability list
handlers (like power management) or the bottom-half handler for pci
operations from the frontend?

Ryan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel