qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH V7 17/29] pci: export functions for cpr


From: Steven Sistare
Subject: Re: [PATCH V7 17/29] pci: export functions for cpr
Date: Wed, 5 Jan 2022 12:22:25 -0500
User-agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.4.1

On 12/22/2021 6:07 PM, Michael S. Tsirkin wrote:
> On Wed, Dec 22, 2021 at 11:05:22AM -0800, Steve Sistare wrote:
>> Export msix_is_pending, msix_init_vector_notifiers, and pci_update_mappings
>> for use by cpr.  No functional change.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> 
> With things like that, I prefer when the API is exported
> together with the patch that uses it.
> This was I can see why we are exporting these APIs.
> Esp wrt pci_update_mappings, it's designed as an
> internal API.

Hi Michael, thanks very much for reviewing these patches.

Serendipitously, I stopped calling pci_update_mappings from vfio code earlier
in the series.  I will revert its scope.

I would prefer to keep this patch separate from the use of these functions in
"vfio-pci cpr part 2 msi", to make the latter smaller and easier to understand.
How about if I say more in this commit message? :

  Export msix_is_pending and msix_init_vector_notifiers for use in vfio cpr.
  Both are needed in the vfio-pci post-load function during cpr-load.
  msix_is_pending is checked to enable the PBA memory region.
  msix_init_vector_notifiers is called to register notifier callbacks, without
  the other side effects of msix_set_vector_notifiers.

- Steve

>> ---
>>  hw/pci/msix.c         | 20 ++++++++++++++------
>>  hw/pci/pci.c          |  3 +--
>>  include/hw/pci/msix.h |  5 +++++
>>  include/hw/pci/pci.h  |  1 +
>>  4 files changed, 21 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/pci/msix.c b/hw/pci/msix.c
>> index ae9331c..73f4259 100644
>> --- a/hw/pci/msix.c
>> +++ b/hw/pci/msix.c
>> @@ -64,7 +64,7 @@ static uint8_t *msix_pending_byte(PCIDevice *dev, int 
>> vector)
>>      return dev->msix_pba + vector / 8;
>>  }
>>  
>> -static int msix_is_pending(PCIDevice *dev, int vector)
>> +int msix_is_pending(PCIDevice *dev, unsigned int vector)
>>  {
>>      return *msix_pending_byte(dev, vector) & msix_pending_mask(vector);
>>  }
>> @@ -579,6 +579,17 @@ static void msix_unset_notifier_for_vector(PCIDevice 
>> *dev, unsigned int vector)
>>      dev->msix_vector_release_notifier(dev, vector);
>>  }
>>  
>> +void msix_init_vector_notifiers(PCIDevice *dev,
>> +                                MSIVectorUseNotifier use_notifier,
>> +                                MSIVectorReleaseNotifier release_notifier,
>> +                                MSIVectorPollNotifier poll_notifier)
>> +{
>> +    assert(use_notifier && release_notifier);
>> +    dev->msix_vector_use_notifier = use_notifier;
>> +    dev->msix_vector_release_notifier = release_notifier;
>> +    dev->msix_vector_poll_notifier = poll_notifier;
>> +}
>> +
>>  int msix_set_vector_notifiers(PCIDevice *dev,
>>                                MSIVectorUseNotifier use_notifier,
>>                                MSIVectorReleaseNotifier release_notifier,
>> @@ -586,11 +597,8 @@ int msix_set_vector_notifiers(PCIDevice *dev,
>>  {
>>      int vector, ret;
>>  
>> -    assert(use_notifier && release_notifier);
>> -
>> -    dev->msix_vector_use_notifier = use_notifier;
>> -    dev->msix_vector_release_notifier = release_notifier;
>> -    dev->msix_vector_poll_notifier = poll_notifier;
>> +    msix_init_vector_notifiers(dev, use_notifier, release_notifier,
>> +                               poll_notifier);
>>  
>>      if ((dev->config[dev->msix_cap + MSIX_CONTROL_OFFSET] &
>>          (MSIX_ENABLE_MASK | MSIX_MASKALL_MASK)) == MSIX_ENABLE_MASK) {
>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>> index e5993c1..0fd21e1 100644
>> --- a/hw/pci/pci.c
>> +++ b/hw/pci/pci.c
>> @@ -225,7 +225,6 @@ static const TypeInfo pcie_bus_info = {
>>  };
>>  
>>  static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
>> -static void pci_update_mappings(PCIDevice *d);
>>  static void pci_irq_handler(void *opaque, int irq_num, int level);
>>  static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom, Error 
>> **);
>>  static void pci_del_option_rom(PCIDevice *pdev);
>> @@ -1366,7 +1365,7 @@ static pcibus_t pci_bar_address(PCIDevice *d,
>>      return new_addr;
>>  }
>>  
>> -static void pci_update_mappings(PCIDevice *d)
>> +void pci_update_mappings(PCIDevice *d)
>>  {
>>      PCIIORegion *r;
>>      int i;
>> diff --git a/include/hw/pci/msix.h b/include/hw/pci/msix.h
>> index 4c4a60c..46606cf 100644
>> --- a/include/hw/pci/msix.h
>> +++ b/include/hw/pci/msix.h
>> @@ -32,6 +32,7 @@ int msix_present(PCIDevice *dev);
>>  bool msix_is_masked(PCIDevice *dev, unsigned vector);
>>  void msix_set_pending(PCIDevice *dev, unsigned vector);
>>  void msix_clr_pending(PCIDevice *dev, int vector);
>> +int msix_is_pending(PCIDevice *dev, unsigned vector);
>>  
>>  int msix_vector_use(PCIDevice *dev, unsigned vector);
>>  void msix_vector_unuse(PCIDevice *dev, unsigned vector);
>> @@ -41,6 +42,10 @@ void msix_notify(PCIDevice *dev, unsigned vector);
>>  
>>  void msix_reset(PCIDevice *dev);
>>  
>> +void msix_init_vector_notifiers(PCIDevice *dev,
>> +                                MSIVectorUseNotifier use_notifier,
>> +                                MSIVectorReleaseNotifier release_notifier,
>> +                                MSIVectorPollNotifier poll_notifier);
>>  int msix_set_vector_notifiers(PCIDevice *dev,
>>                                MSIVectorUseNotifier use_notifier,
>>                                MSIVectorReleaseNotifier release_notifier,
>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
>> index e7cdf2d..cc63dd4 100644
>> --- a/include/hw/pci/pci.h
>> +++ b/include/hw/pci/pci.h
>> @@ -910,5 +910,6 @@ extern const VMStateDescription vmstate_pci_device;
>>  
>>  MSIMessage pci_get_msi_message(PCIDevice *dev, int vector);
>>  void pci_set_power(PCIDevice *pci_dev, bool state);
>> +void pci_update_mappings(PCIDevice *d);
>>  
>>  #endif
>> -- 
>> 1.8.3.1
> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]