linux-kernel - Re: [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on Hip06

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9178320.n4yHmfyPA3@wuerfel>
Date:   Thu, 22 Sep 2016 14:14:31 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Gabriele Paoloni <gabriele.paoloni@...wei.com>
Cc:     zhichang <zhichang.yuan02@...il.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "lorenzo.pieralisi@....com" <lorenzo.pieralisi@....com>,
        "minyard@....org" <minyard@....org>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        John Garry <john.garry@...wei.com>,
        "will.deacon@....com" <will.deacon@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Yuanzhichang <yuanzhichang@...ilicon.com>,
        Linuxarm <linuxarm@...wei.com>,
        "xuwei (O)" <xuwei5@...ilicon.com>,
        "linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
        "benh@...nel.crashing.org" <benh@...nel.crashing.org>,
        "zourongrong@...il.com" <zourongrong@...il.com>,
        "liviu.dudau@....com" <liviu.dudau@....com>,
        "kantyzc@....com" <kantyzc@....com>
Subject: Re: [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on Hip06

On Thursday, September 22, 2016 11:55:45 AM CEST Gabriele Paoloni wrote:
> > > I think extending of_empty_ranges_quirk() may be a reasonable
> > solution.
> > > What do you think Arnd?
> > 
> > I don't really like that idea, that quirk is meant to work around
> > broken DTs, but we can just make the DT valid and implement the
> > code properly.
> 
> Ok  I understand your point where it is not right to use of_empty_ranges_quirk()
> As a quirk is used to work around broken HW or broken FW (as in this case)
> rather than to fix code
> 
> What about the following? I think adding the check you suggested next to
> of_empty_ranges_quirk() is adding the case we need in the right point (thus
> avoiding any duplication)
>  
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -457,6 +457,15 @@ static struct of_bus *of_match_bus(struct device_node *np)
>         return NULL;
>  }
>  
> +static inline int of_isa_indirect_io(struct device_node *np)
> +{
> +       /*
> +        * check if the current node is an isa bus and if indirectio operation
> +        * are registered
> +        */
> +       return (of_bus_isa_match(np) && arm64_extio_ops);
> +}
> +
>  static int of_empty_ranges_quirk(struct device_node *np)
>  {
>         if (IS_ENABLED(CONFIG_PPC)) {
> @@ -503,7 +512,7 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
>          * This code is only enabled on powerpc. --gcl
>          */
>         ranges = of_get_property(parent, rprop, &rlen);
> -       if (ranges == NULL && !of_empty_ranges_quirk(parent)) {
> +       if (ranges == NULL && !of_empty_ranges_quirk(parent) && !of_isa_indirect_io(parent)) {
>                 pr_debug("OF: no ranges; cannot translate\n");
>                 return 1;
>         }

I don't see what effect that would have. What do you want to
achieve with this?

I think all we need from this function is to return '1' if
we hit an ISA I/O window, and that should happen for the two
interesting cases, either no 'ranges' at all, or no translation
for the range in question, so that __of_translate_address can
return OF_BAD_ADDR, and we can enter the special case
handling in the caller, that handles it like

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 02b2903fe9d2..a18d96843fae 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -685,17 +685,24 @@ static int __of_address_to_resource(struct device_node *dev,
 	if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0)
 		return -EINVAL;
 	taddr = of_translate_address(dev, addrp);
-	if (taddr == OF_BAD_ADDR)
-		return -EINVAL;
 	memset(r, 0, sizeof(struct resource));
+
 	if (flags & IORESOURCE_IO) {
 		unsigned long port;
-		port = pci_address_to_pio(taddr);
+
+		if (taddr == OF_BAD_ADDR)
+			port = arch_of_address_to_pio(dev, addrp)
+		else
+			port = pci_address_to_pio(taddr);
+
 		if (port == (unsigned long)-1)
 			return -EINVAL;
 		r->start = port;
 		r->end = port + size - 1;
 	} else {
+		if (taddr == OF_BAD_ADDR)
+			return -EINVAL;
+
 		r->start = taddr;
 		r->end = taddr + size - 1;
 	}



	Arnd