[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 3/3] passthrough: allow to suppress SERR and PERR signaling altogether
>>> On 07.04.14 at 15:17, <andrew.cooper3@xxxxxxxxxx> wrote: > On 07/04/14 14:05, Jan Beulich wrote: >>>>> On 07.04.14 at 14:47, <andrew.cooper3@xxxxxxxxxx> wrote: >>> On 03/04/14 10:41, Jan Beulich wrote: >>>> + if ( val & PCI_STATUS_CHECK ) >>>> + { >>>> + printk(XENLOG_INFO "%04x:%02x:%02x.%u status %04x\n", >>>> + seg, bus, dev, func, val); >>> What is the purpose of this printk? From the text alone it is not obvious. >> It's simply to have an indication that the status register was written >> (and that certain bits may have got cleared). > > Then at the very least it should be ..."status %04x -> %04x\n", .... > val, val & PCI_STATUS_CHECK) to identify which status bits are being > cleared. This obviously makes sense only when also changing the write operation to use val & PCI_STATUS_CHECK, and then it would seem to make more sense to print val & ~PCI_STATUS_CHECK as the second value. >>>> + pci_conf_write16(seg, bus, dev, func, PCI_STATUS, val); >>> I dont think this code has any right to clear status bits other than the >>> ones it is checking for, so the write should be "val & PCI_STATUS_CHECK" >> Hmm, the intention is to clear all status fields that can be cleared, and >> the if() around the write is just to avoid the printk() and the write if >> possible. PCI_STATUS_CHECK already includes all changeable bits, and >> I'd expect any of the few that are currently reserved to get added >> here, should they attain a meaning of a writable one. > > For forward compatibility, we must not assume anything about currently > reserved bits which Xen doesn't know about. In general this is correct; I'm not so certain in the particular case here. For one, the code doesn't get used by default, but only when asked for on the command line. If the code was found to have a problem, just don't specify the command line option. And then, as said before, the intention is to get the status register into a clean state. It's a status register, so we're not going to corrupt device state by clearing a few too many bits, and we log which bits were set at the time we did the clearing. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |