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

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Thu, Nov 21, 2013 at 04:49:33PM -0600, Aravind Gopalakrishnan wrote:
> On 11/20/2013 2:10 AM, Jan Beulich wrote:
> >>>>On 20.11.13 at 01:53, Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com> wrote:
> >>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).
> >
> >>But-
> >>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)?
> >
> >Jan
> >
> >
> 
> 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..

What is the stack trace? So that at least I know what to look for
if somebody mentions this?

Thanks
> 
> 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..)
> 
> -Aravind.
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel