Re: [Qemu-devel] [PATCH qemu v2 0/4] spapr_pci, vfio: NVIDIA V100 + POWER9 passthrough

[Alexey Kardashevskiy]

On 15/02/2019 14:24, David Gibson wrote:
> On Fri, Feb 15, 2019 at 11:35:02AM +1100, Alexey Kardashevskiy wrote:
>>
>>
>> On 15/02/2019 10:37, Alex Williamson wrote:
>>> On Thu, 14 Feb 2019 16:21:40 +1100
>>> Alexey Kardashevskiy <address@hidden> wrote:
>>>
>>>> This is for passing through NVIDIA V100 GPUs on POWER9 systems.
>>>>
>>>> This implements a subdriver for NVIDIA V100 GPU with coherent memory and
>>>> NPU/ATS support available in the POWER9 CPU.
>>>>
>>>> 1/4 is a preparation for bigger DMA windows.
>>>> 2/4 is a small cleanup.
>>>>
>>>> Here is the kernel driver:
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/vfio/pci?h=v5.0-rc6&id=7f92891778dff62303c070ac81de7b7d80de331a
>>>>
>>>> SLOF changes already went in.
>>>>
>>>> This depends on "pci: Move NVIDIA vendor id to the rest of ids" (posted 
>>>> separately).
>>>
>>> TBH, I'm not sure it was the best idea to let it live or die on it's
>>> own when it now creates a build dependency for this series.
>> I am sure that patch is disgustingly primitive and can make it to
>> upstream in just one click and the rest of the series will take more
>> time anyway (always does :) ).
>>
>>>> This is based on sha1
>>>> 1ea6057 Mark Cave-Ayland "mac_newworld: change default NIC to sungem for 
>>>> mac99 machine".
>>>
>>> Perhaps this is why it doesn't apply cleanly against qemu.git.  Are
>>> there dependencies we need to wait for in the ppc tree as well?
>>
>> There are few changes in spapr_pci so conflicts are possible, every time
>> when David updates his tree and I rebase I get some minor ones.
>>
>>>> Please comment. Thanks.
>>>
>>> Besides the build dependency on PCI_VENDOR_ID_NVIDIA, I also get this:
>>>
>>> .../qemu.git/hw/vfio/spapr.c: In function ‘vfio_spapr_create_window’:
>>> .../qemu.git/hw/vfio/spapr.c:212:9: error: ‘ret’ may be used uninitialized 
>>> in this function [-Werror=maybe-uninitialized]
>>>          error_report("Failed to create a window, ret = %d (%m)", ret);
>>>          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> cc1: all warnings being treated as errors
>>
>> Agrh. How exactly do you make them errors, not warnings? I get no
>> warning/error with --disable-werror but not having  --disable-werror
>> prints warnings, not errors so it does not fail the build and easy to miss.
> 
> Are you sure it's printing warnings with --enable-werror?  Otherwise
> it sounds like you just have a compiler version that's not picking
> this up.

Ah, here is my mistake - I thought not having --disable-werror means
that it is enabled as ./configure does not advertise  "--enable-werror":

[fstn1-p1 qemu]$ ./configure --help | grep werror
  --disable-werror         disable compilation abort on warning


Apparently it is a tri-state flag :)


> 
>>> Is this series expected to go in through the ppc branch given the bulk
>>> of the changes are there?  Thanks,
>>
>> I think so.
>>
>>
> 

-- 
Alexey