On 9/2/23 21:08, Vladimir Sementsov-Ogievskiy wrote:
In pcie_cap_slot_write_config() we check for PCI_EXP_SLTCTL_PWR_OFF in
a bad form. We should distinguish PCI_EXP_SLTCTL_PWR which is a "mask"
and PCI_EXP_SLTCTL_PWR_OFF which is value for that mask.
Better code is in pcie_cap_slot_unplug_request_cb() and in
pcie_cap_update_power(). Let's use same pattern everywhere. To simplify
things add also a helper.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
---
hw/pci/pcie.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
+static bool pcie_sltctl_powered_off(uint16_t sltctl)
+{
+ return (sltctl & PCI_EXP_SLTCTL_PCC) == PCI_EXP_SLTCTL_PWR_OFF &&
+ (sltctl & PCI_EXP_SLTCTL_PIC) == PCI_EXP_SLTCTL_PWR_IND_OFF;
Matter of taste (it is harder to miss the &&):
return (sltctl & PCI_EXP_SLTCTL_PIC) == PCI_EXP_SLTCTL_PWR_IND_OFF
&& (sltctl & PCI_EXP_SLTCTL_PCC) == PCI_EXP_SLTCTL_PWR_OFF;
@@ -770,10 +775,9 @@ void pcie_cap_slot_write_config(PCIDevice *dev,
* this is a work around for guests that overwrite
* control of powered off slots before powering them on.
*/
- if ((sltsta & PCI_EXP_SLTSTA_PDS) && (val & PCI_EXP_SLTCTL_PCC) &&
- (val & PCI_EXP_SLTCTL_PIC) == PCI_EXP_SLTCTL_PWR_IND_OFF &&
- (!(old_slt_ctl & PCI_EXP_SLTCTL_PCC) ||
- (old_slt_ctl & PCI_EXP_SLTCTL_PIC) != PCI_EXP_SLTCTL_PWR_IND_OFF)) {
+ if ((sltsta & PCI_EXP_SLTSTA_PDS) && pcie_sltctl_powered_off(val) &&
+ !pcie_sltctl_powered_off(old_slt_ctl))
Certainly simpler!
Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
