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

Re: [Xen-devel] [PATCH V2] ns16550: Add support for UART present in Broadcom TruManage capable NetXtreme chips

On 11/20/2013 2:10 AM, Jan Beulich wrote:
On 20.11.13 at 01:53, Aravind Gopalakrishnan <aravind.gopalakrishnan@xxxxxxx> 
Looks like the arguments I was passing in to 'iomem_deny_access' was
incorrect before (apologies)
(I just had to use paddr_to_pfn() to get PAGE_SHIFT-ed value)
I tried with the proper (page shifted) values, but it breaks Xen
throwing the message:

(XEN) mm.c:785:d0 Non-privileged (0) attempt to map I/O space
Xen should not be affected by this message appearing; Dom0
likely would be in one way or another.

The reason for this is -  dom0 sees the UART device and tries to
configure it at the bar value (which is blocked by Xen)
which means pci_hide_device() is not functioning as expected..(again,
not sure if I am missing something..).
Then you didn't understand the purpose of pci_hide_device(),
yet I would have expected you to have looked at commit e46ea4d4
("PCI: don't allow guest assignment of devices used by Xen") in this
context: Such devices are unavailable for assignment to a DomU,
but visible as usual to Dom0 (and nothing prevents Dom0 from
assigning the device e.g. new BAR values - pci_ro_device() would -,
and hence using iomem_deny_access() is pointless/wrong).

Could this be due to the fact that this is a multifunction device and
the UART is only a subfunction?
Multi-function in the usual sense? If so, all the BARs on that
function are only to be used by that function... Or do you perhaps
mean a function in the PCI sense providing more functionality than
just the serial port (as in many other combined serial/parallel or
multi-port serial add in cards)?


I meant multi-function as in latter sense (provides more than a serial port). Here is snapshot of lspci for clarity -

02:00.0 Ethernet controller [0200]: Broadcom Corporation NetXtreme BCM5725 Gigabit Ethernet PCIe [14e4:1643] (rev 10) 02:00.5 Communication controller [0780]: Broadcom Corporation Device [14e4:160a] (rev 01) 02:00.6 IPMI SMIC interface [0c07]: Broadcom Corporation Device [14e4:16b9] (rev 10)

Anyway, I have now reworked the code such that Xen hides the MMIO region from (io_base+PAGE_SIZE) to end of the region.
([    0.000000] Xen: [mem 0x00000000d0031000-0x00000000d003ffff] reserved)
This would (in some extent) alleviate conflicts in case Dom0 interferes with Xen's control of the device as we are hiding all
possible PAGE_SIZE'd chunks of the MMIO region..
Since we can't hide the device from dom0, dom0 sees the device and tries to 'ioremap' at the above said regions, but fails. Although it does not break Xen, dom0 throws a stack trace with a warning message. dom0 continues to boot fine..

Also, to your comments on V3 - (Sorry I have to do this here as my mail client seems to have lost the V3 thread..) I have fixed the code in accordance to your comments except for the '(-len)' suggestion. Reason being -
It does not work all the time. Example: (for IO case)
after applying (len &= PCI_BASE_ADDRESS_IO_MASK),
len = fff8;
len = -(len) would give you ffff0008 and you will need to mask off higher 16 bits again..

Instead, I have kept length calculation consistent with what linux does and extended that to IO case..

Sending the changes out as V4.
(Tested the code changes with BCM5725 and an IO based UART and verified to be working correctly..)


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.