diff mbox

pci: Translate PCI addresses to host addresses at the end of map-in

Message ID 1499951274-25709-1-git-send-email-thuth@redhat.com
State Superseded
Headers show

Commit Message

Thomas Huth July 13, 2017, 1:07 p.m. UTC
Currently, it is not possible to use VGA devices attached to a
PCI bridge on board-qemu, e.g. by starting QEMU like this:

 qemu-system-ppc64 -nodefaults -device pci-bridge,id=br1,chassis_nr=1 \
        -serial mon:stdio -device VGA,id=video,bus=br1,addr=1

One of the problems is the missing translate-address at the end
of the map-in function of the bridge - which was already marked
as a TODO, but apparently has never been enabled. So let's do
that now!

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 slof/fs/pci-config-bridge.fs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Thomas Huth July 13, 2017, 1:18 p.m. UTC | #1
On 13.07.2017 15:07, Thomas Huth wrote:
> Currently, it is not possible to use VGA devices attached to a
> PCI bridge on board-qemu, e.g. by starting QEMU like this:
> 
>  qemu-system-ppc64 -nodefaults -device pci-bridge,id=br1,chassis_nr=1 \
>         -serial mon:stdio -device VGA,id=video,bus=br1,addr=1
> 
> One of the problems is the missing translate-address at the end
> of the map-in function of the bridge - which was already marked
> as a TODO, but apparently has never been enabled. So let's do
> that now!

The second problem is uglier, and I'm not quite sure what's the best way
to fix it yet: The first BAR of the VGA device is a 32-bit prefetchable
BAR, which is initialized via assign-mem32-bar and thus using the value
from pci-next-mem. Now the bridge has only two windows, one for 64-bit
prefetchable memory (configured via pci-next-mem64 here), and one for
32-bit non-prefetchable memory (configured via pci-next-mmio). So the
VGA device ends up with a memory region that is not covered by the bridge.

Should we maybe rather configure 32-bit prefetchable BARs via
pci-next-mmio instead of pci-next-mem if the 64-bit pci-next-mem64 is
available and used for bridges? Or has anybody any other suggestions how
to fix this properly?

 Thomas
Segher Boessenkool July 13, 2017, 10:31 p.m. UTC | #2
On Thu, Jul 13, 2017 at 03:18:23PM +0200, Thomas Huth wrote:
> On 13.07.2017 15:07, Thomas Huth wrote:
> > Currently, it is not possible to use VGA devices attached to a
> > PCI bridge on board-qemu, e.g. by starting QEMU like this:
> > 
> >  qemu-system-ppc64 -nodefaults -device pci-bridge,id=br1,chassis_nr=1 \
> >         -serial mon:stdio -device VGA,id=video,bus=br1,addr=1
> > 
> > One of the problems is the missing translate-address at the end
> > of the map-in function of the bridge - which was already marked
> > as a TODO, but apparently has never been enabled. So let's do
> > that now!
> 
> The second problem is uglier, and I'm not quite sure what's the best way
> to fix it yet: The first BAR of the VGA device is a 32-bit prefetchable
> BAR, which is initialized via assign-mem32-bar and thus using the value
> from pci-next-mem. Now the bridge has only two windows, one for 64-bit
> prefetchable memory (configured via pci-next-mem64 here), and one for
> 32-bit non-prefetchable memory (configured via pci-next-mmio). So the
> VGA device ends up with a memory region that is not covered by the bridge.
> 
> Should we maybe rather configure 32-bit prefetchable BARs via
> pci-next-mmio instead of pci-next-mem if the 64-bit pci-next-mem64 is
> available and used for bridges? Or has anybody any other suggestions how
> to fix this properly?

That, or set up the prefetchable window to (partially) be below 4GB?


Segher
Alexey Kardashevskiy July 14, 2017, 4:59 a.m. UTC | #3
On 13/07/17 23:18, Thomas Huth wrote:
> On 13.07.2017 15:07, Thomas Huth wrote:
>> Currently, it is not possible to use VGA devices attached to a
>> PCI bridge on board-qemu, e.g. by starting QEMU like this:
>>
>>  qemu-system-ppc64 -nodefaults -device pci-bridge,id=br1,chassis_nr=1 \
>>         -serial mon:stdio -device VGA,id=video,bus=br1,addr=1
>>
>> One of the problems is the missing translate-address at the end
>> of the map-in function of the bridge - which was already marked
>> as a TODO, but apparently has never been enabled. So let's do
>> that now!
> 
> The second problem is uglier, and I'm not quite sure what's the best way
> to fix it yet: The first BAR of the VGA device is a 32-bit prefetchable
> BAR, which is initialized via assign-mem32-bar and thus using the value
> from pci-next-mem. Now the bridge has only two windows, one for 64-bit
> prefetchable memory (configured via pci-next-mem64 here), and one for
> 32-bit non-prefetchable memory (configured via pci-next-mmio). So the
> VGA device ends up with a memory region that is not covered by the bridge.


VGA should be under 32bit non-prefetchable window, what is the problem with
putting it there? VGA's BAR being prefetchable means that a bridge can
prefetch from it but it does not have to, no?


> Should we maybe rather configure 32-bit prefetchable BARs via
> pci-next-mmio instead of pci-next-mem if the 64-bit pci-next-mem64 is
> available and used for bridges? Or has anybody any other suggestions how
> to fix this properly?
Thomas Huth July 17, 2017, 10:16 a.m. UTC | #4
On 13.07.2017 15:07, Thomas Huth wrote:
> Currently, it is not possible to use VGA devices attached to a
> PCI bridge on board-qemu, e.g. by starting QEMU like this:
> 
>  qemu-system-ppc64 -nodefaults -device pci-bridge,id=br1,chassis_nr=1 \
>         -serial mon:stdio -device VGA,id=video,bus=br1,addr=1
> 
> One of the problems is the missing translate-address at the end
> of the map-in function of the bridge - which was already marked
> as a TODO, but apparently has never been enabled. So let's do
> that now!
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  slof/fs/pci-config-bridge.fs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/slof/fs/pci-config-bridge.fs b/slof/fs/pci-config-bridge.fs
> index 1efbcd8..4169aa8 100644
> --- a/slof/fs/pci-config-bridge.fs
> +++ b/slof/fs/pci-config-bridge.fs
> @@ -73,7 +73,7 @@
>        nip
>     THEN
>     F NOT AND                            \ Clear indicator bits
> -   \ TODO: Use translate-address here!
> +   translate-my-address
>  ;

Looking at my patch again, I think it is cleaner if I do a "my-phandle
swap translate-address" here ... so please ignore this version, I'll
send a v2 instead.

 Thomas
Thomas Huth July 17, 2017, 11:17 a.m. UTC | #5
On 17.07.2017 12:16, Thomas Huth wrote:
> On 13.07.2017 15:07, Thomas Huth wrote:
[...]
>> diff --git a/slof/fs/pci-config-bridge.fs b/slof/fs/pci-config-bridge.fs
>> index 1efbcd8..4169aa8 100644
>> --- a/slof/fs/pci-config-bridge.fs
>> +++ b/slof/fs/pci-config-bridge.fs
>> @@ -73,7 +73,7 @@
>>        nip
>>     THEN
>>     F NOT AND                            \ Clear indicator bits
>> -   \ TODO: Use translate-address here!
>> +   translate-my-address
>>  ;
> 
> Looking at my patch again, I think it is cleaner if I do a "my-phandle
> swap translate-address" here ... so please ignore this version, I'll
> send a v2 instead.

Ah, well, please ignore my "please ignore" message above ;-)
Looks like "my-phandle" does not work here - since it is set to the
bridge and not to the device here. So "translate-my-address" is the
right thing to do here and the above version of my patch is fine.

 Thomas
Alexey Kardashevskiy July 20, 2017, 6:46 a.m. UTC | #6
On 13/07/17 23:07, Thomas Huth wrote:
> Currently, it is not possible to use VGA devices attached to a
> PCI bridge on board-qemu, e.g. by starting QEMU like this:
> 
>  qemu-system-ppc64 -nodefaults -device pci-bridge,id=br1,chassis_nr=1 \
>         -serial mon:stdio -device VGA,id=video,bus=br1,addr=1
> 
> One of the problems is the missing translate-address at the end
> of the map-in function of the bridge - which was already marked
> as a TODO, but apparently has never been enabled. So let's do
> that now!
> 

Thanks, applied.

I wonder why did you do that at the time when you put that "TODO" comment? :)


> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  slof/fs/pci-config-bridge.fs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/slof/fs/pci-config-bridge.fs b/slof/fs/pci-config-bridge.fs
> index 1efbcd8..4169aa8 100644
> --- a/slof/fs/pci-config-bridge.fs
> +++ b/slof/fs/pci-config-bridge.fs
> @@ -73,7 +73,7 @@
>        nip
>     THEN
>     F NOT AND                            \ Clear indicator bits
> -   \ TODO: Use translate-address here!
> +   translate-my-address
>  ;
>  
>  : map-out ( virt size -- )
>
Thomas Huth July 20, 2017, 7:33 a.m. UTC | #7
On 20.07.2017 08:46, Alexey Kardashevskiy wrote:
> On 13/07/17 23:07, Thomas Huth wrote:
>> Currently, it is not possible to use VGA devices attached to a
>> PCI bridge on board-qemu, e.g. by starting QEMU like this:
>>
>>  qemu-system-ppc64 -nodefaults -device pci-bridge,id=br1,chassis_nr=1 \
>>         -serial mon:stdio -device VGA,id=video,bus=br1,addr=1
>>
>> One of the problems is the missing translate-address at the end
>> of the map-in function of the bridge - which was already marked
>> as a TODO, but apparently has never been enabled. So let's do
>> that now!
>>
> 
> Thanks, applied.
> 
> I wonder why did you do that at the time when you put that "TODO" comment? :)

I think I did not write that part of the code ... but IIRC the PCI code
was written before we had the translate-address function in a usable
shape, so the TODO comment was all that could have been done at that
point in time. Later we likely forgot about it since it was not
necessary on JS2x.

 Thomas
Thomas Huth July 20, 2017, 8:07 a.m. UTC | #8
On 20.07.2017 09:56, Alexey Kardashevskiy wrote:
> On 20/07/17 17:33, Thomas Huth wrote:
>> On 20.07.2017 08:46, Alexey Kardashevskiy wrote:
>>> On 13/07/17 23:07, Thomas Huth wrote:
>>>> Currently, it is not possible to use VGA devices attached to a
>>>> PCI bridge on board-qemu, e.g. by starting QEMU like this:
>>>>
>>>>  qemu-system-ppc64 -nodefaults -device pci-bridge,id=br1,chassis_nr=1 \
>>>>         -serial mon:stdio -device VGA,id=video,bus=br1,addr=1
>>>>
>>>> One of the problems is the missing translate-address at the end
>>>> of the map-in function of the bridge - which was already marked
>>>> as a TODO, but apparently has never been enabled. So let's do
>>>> that now!
>>>>
>>>
>>> Thanks, applied.
>>>
>>> I wonder why did you do that at the time when you put that "TODO" comment? :)
>>
>> I think I did not write that part of the code ...
> 
> https://github.com/aik/SLOF/commit/de5b143a ;)
> 
>> but IIRC the PCI code
>> was written before we had the translate-address function in a usable
>> shape,
> 
> slof/fs/translate.fs's translate-address is there since the initial commit ;)

Ooops, looks like my memory completely failed here. It's true that the
PCI code has been originally written before translate-address had been
implemented, but seems like I completely forgot about that map-in patch
that I added there later, sorry! Not sure why I did not use
translate-address here right from the start - maybe simply because it
was just not necessary on JS2x - or maybe because there are some other
bugs left in the JS2x code that caused trouble here ... hard to tell
without having a way to test the JS2x ATI code again...

 Thomas
diff mbox

Patch

diff --git a/slof/fs/pci-config-bridge.fs b/slof/fs/pci-config-bridge.fs
index 1efbcd8..4169aa8 100644
--- a/slof/fs/pci-config-bridge.fs
+++ b/slof/fs/pci-config-bridge.fs
@@ -73,7 +73,7 @@ 
       nip
    THEN
    F NOT AND                            \ Clear indicator bits
-   \ TODO: Use translate-address here!
+   translate-my-address
 ;
 
 : map-out ( virt size -- )