Skip to content

Commit d6bf060

Browse files
donshengwenlingz
donsheng
authored and
wenlingz
committed
HV: remove redundant function calling
The caller function has already done the checking to make sure the req is targeted for the called functions, so there is no need to do the same checking in called functions. Remove calling of is_bar_offset() in vdev_pt_read_cfg/vdev_pt_write_cfg: In vpci.c's vpci_read_pt_dev_cfg and vpci_write_dev_cfg, vbar_access is called first to make sure the req is targed for vdev pt (vbar emulation) before dispatching the request to vdev_pt_read_cfg/vdev_pt_write_cfg, so there is no need to call is_bar_offset() again to do the same checking in vdev_pt_read_cfg/vdev_pt_write_cfg. The same goes for msicap_access/msixcap_access vbar_access should only check if the req is for bar access, it should not care about whether the bar access is 4 bytes or 4 bytes aligned. The called function vdev_pt_write_vbar will check and ignore the write access if it is not 4 bytes or 4 bytes aligned, although this is counted as a bar access. vdev_pt_read_vbar will check if the read access is 4 bytes or 4 bytes aligned, although this is counted as a bar access, set read value (*val) to -1 if the access is not 4 bytes (or aligned). Tracked-On: #3475 Signed-off-by: dongshen <dongsheng.x.zhang@intel.com> Acked-by: Eddie Dong <eddie.dong@intel.com>
1 parent c175141 commit d6bf060

File tree

5 files changed

+53
-74
lines changed

5 files changed

+53
-74
lines changed

‎hypervisor/dm/vpci/pci_pt.c

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -131,14 +131,14 @@ static uint64_t get_pbar_base(const struct pci_pdev *pdev, uint32_t idx)
131131
*/
132132
int32_t vdev_pt_read_cfg(const struct pci_vdev *vdev, uint32_t offset, uint32_t bytes, uint32_t *val)
133133
{
134-
int32_t ret = -ENODEV;
135-
136-
if (is_bar_offset(vdev->nr_bars, offset)) {
134+
/* bar access must be 4 bytes and offset must also be 4 bytes aligned */
135+
if ((bytes == 4U) && ((offset & 0x3U) == 0U)) {
137136
*val = pci_vdev_read_cfg(vdev, offset, bytes);
138-
ret = 0;
137+
} else {
138+
*val = ~0U;
139139
}
140140

141-
return ret;
141+
return 0;
142142
}
143143

144144
/**
@@ -442,15 +442,12 @@ static void vdev_pt_write_vbar(struct pci_vdev *vdev, uint32_t offset, uint32_t
442442
*/
443443
int32_t vdev_pt_write_cfg(struct pci_vdev *vdev, uint32_t offset, uint32_t bytes, uint32_t val)
444444
{
445-
int32_t ret = -ENODEV;
446-
447-
/* bar write access must be 4 bytes and offset must also be 4 bytes aligned*/
448-
if (is_bar_offset(vdev->nr_bars, offset) && (bytes == 4U) && ((offset & 0x3U) == 0U)) {
445+
/* bar write access must be 4 bytes and offset must also be 4 bytes aligned */
446+
if ((bytes == 4U) && ((offset & 0x3U) == 0U)) {
449447
vdev_pt_write_vbar(vdev, offset, val);
450-
ret = 0;
451448
}
452449

453-
return ret;
450+
return 0;
454451
}
455452

456453
/**

‎hypervisor/dm/vpci/vmsi.c

Lines changed: 21 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -103,56 +103,46 @@ static int32_t vmsi_remap(const struct pci_vdev *vdev, bool enable)
103103
*/
104104
int32_t vmsi_read_cfg(const struct pci_vdev *vdev, uint32_t offset, uint32_t bytes, uint32_t *val)
105105
{
106-
int32_t ret = -ENODEV;
107-
108106
/* For PIO access, we emulate Capability Structures only */
109-
if (msicap_access(vdev, offset)) {
110-
*val = pci_vdev_read_cfg(vdev, offset, bytes);
111-
ret = 0;
112-
}
107+
*val = pci_vdev_read_cfg(vdev, offset, bytes);
113108

114-
return ret;
109+
return 0;
115110
}
116111

117112
/**
113+
* @brief Writing MSI Capability Structure
114+
*
118115
* @pre vdev != NULL
119116
*/
120117
int32_t vmsi_write_cfg(struct pci_vdev *vdev, uint32_t offset, uint32_t bytes, uint32_t val)
121118
{
122119
bool message_changed = false;
123120
bool enable;
124121
uint32_t msgctrl;
125-
int32_t ret = -ENODEV;
126-
127-
/* Writing MSI Capability Structure */
128-
if (msicap_access(vdev, offset)) {
129122

130-
/* Save msgctrl for comparison */
131-
msgctrl = pci_vdev_read_cfg(vdev, vdev->msi.capoff + PCIR_MSI_CTRL, 2U);
123+
/* Save msgctrl for comparison */
124+
msgctrl = pci_vdev_read_cfg(vdev, vdev->msi.capoff + PCIR_MSI_CTRL, 2U);
132125

133-
/* Either Message Data or message Addr is being changed */
134-
if (((offset - vdev->msi.capoff) >= PCIR_MSI_ADDR) && (val != pci_vdev_read_cfg(vdev, offset, bytes))) {
135-
message_changed = true;
136-
}
126+
/* Either Message Data or message Addr is being changed */
127+
if (((offset - vdev->msi.capoff) >= PCIR_MSI_ADDR) && (val != pci_vdev_read_cfg(vdev, offset, bytes))) {
128+
message_changed = true;
129+
}
137130

138-
/* Write to vdev */
139-
pci_vdev_write_cfg(vdev, offset, bytes, val);
131+
/* Write to vdev */
132+
pci_vdev_write_cfg(vdev, offset, bytes, val);
140133

141-
/* Do remap if MSI Enable bit is being changed */
142-
if (((offset - vdev->msi.capoff) == PCIR_MSI_CTRL) &&
143-
(((msgctrl ^ val) & PCIM_MSICTRL_MSI_ENABLE) != 0U)) {
144-
enable = ((val & PCIM_MSICTRL_MSI_ENABLE) != 0U);
145-
(void)vmsi_remap(vdev, enable);
146-
} else {
147-
if (message_changed && ((msgctrl & PCIM_MSICTRL_MSI_ENABLE) != 0U)) {
148-
(void)vmsi_remap(vdev, true);
149-
}
134+
/* Do remap if MSI Enable bit is being changed */
135+
if (((offset - vdev->msi.capoff) == PCIR_MSI_CTRL) &&
136+
(((msgctrl ^ val) & PCIM_MSICTRL_MSI_ENABLE) != 0U)) {
137+
enable = ((val & PCIM_MSICTRL_MSI_ENABLE) != 0U);
138+
(void)vmsi_remap(vdev, enable);
139+
} else {
140+
if (message_changed && ((msgctrl & PCIM_MSICTRL_MSI_ENABLE) != 0U)) {
141+
(void)vmsi_remap(vdev, true);
150142
}
151-
152-
ret = 0;
153143
}
154144

155-
return ret;
145+
return 0;
156146
}
157147

158148
/**

‎hypervisor/dm/vpci/vmsix.c

Lines changed: 18 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -168,51 +168,43 @@ static int32_t vmsix_remap_one_entry(const struct pci_vdev *vdev, uint32_t index
168168
*/
169169
int32_t vmsix_read_cfg(const struct pci_vdev *vdev, uint32_t offset, uint32_t bytes, uint32_t *val)
170170
{
171-
int32_t ret = -ENODEV;
172171
/* For PIO access, we emulate Capability Structures only */
172+
*val = pci_vdev_read_cfg(vdev, offset, bytes);
173173

174-
if (msixcap_access(vdev, offset)) {
175-
*val = pci_vdev_read_cfg(vdev, offset, bytes);
176-
ret = 0;
177-
}
178-
179-
return ret;
174+
return 0;
180175
}
181176

182177
/**
178+
* @brief Writing MSI-X Capability Structure
179+
*
183180
* @pre vdev != NULL
184181
* @pre vdev->pdev != NULL
185182
*/
186183
int32_t vmsix_write_cfg(struct pci_vdev *vdev, uint32_t offset, uint32_t bytes, uint32_t val)
187184
{
188185
uint32_t msgctrl;
189-
int32_t ret = -ENODEV;
190186

191-
/* Writing MSI-X Capability Structure */
192-
if (msixcap_access(vdev, offset)) {
193-
msgctrl = pci_vdev_read_cfg(vdev, vdev->msix.capoff + PCIR_MSIX_CTRL, 2U);
187+
msgctrl = pci_vdev_read_cfg(vdev, vdev->msix.capoff + PCIR_MSIX_CTRL, 2U);
194188

195-
/* Write to vdev */
196-
pci_vdev_write_cfg(vdev, offset, bytes, val);
189+
/* Write to vdev */
190+
pci_vdev_write_cfg(vdev, offset, bytes, val);
197191

198-
/* Writing Message Control field? */
199-
if ((offset - vdev->msix.capoff) == PCIR_MSIX_CTRL) {
200-
if (((msgctrl ^ val) & PCIM_MSIXCTRL_MSIX_ENABLE) != 0U) {
201-
if ((val & PCIM_MSIXCTRL_MSIX_ENABLE) != 0U) {
202-
(void)vmsix_remap(vdev, true);
203-
} else {
204-
(void)vmsix_remap(vdev, false);
205-
}
192+
/* Writing Message Control field? */
193+
if ((offset - vdev->msix.capoff) == PCIR_MSIX_CTRL) {
194+
if (((msgctrl ^ val) & PCIM_MSIXCTRL_MSIX_ENABLE) != 0U) {
195+
if ((val & PCIM_MSIXCTRL_MSIX_ENABLE) != 0U) {
196+
(void)vmsix_remap(vdev, true);
197+
} else {
198+
(void)vmsix_remap(vdev, false);
206199
}
200+
}
207201

208-
if (((msgctrl ^ val) & PCIM_MSIXCTRL_FUNCTION_MASK) != 0U) {
209-
pci_pdev_write_cfg(vdev->pdev->bdf, offset, 2U, val);
210-
}
202+
if (((msgctrl ^ val) & PCIM_MSIXCTRL_FUNCTION_MASK) != 0U) {
203+
pci_pdev_write_cfg(vdev->pdev->bdf, offset, 2U, val);
211204
}
212-
ret = 0;
213205
}
214206

215-
return ret;
207+
return 0;
216208
}
217209

218210
/**

‎hypervisor/dm/vpci/vpci.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ static void vpci_deinit_pt_dev(struct pci_vdev *vdev)
354354
static int32_t vpci_write_pt_dev_cfg(struct pci_vdev *vdev, uint32_t offset,
355355
uint32_t bytes, uint32_t val)
356356
{
357-
if (vbar_access(vdev, offset, bytes)) {
357+
if (vbar_access(vdev, offset)) {
358358
(void)vdev_pt_write_cfg(vdev, offset, bytes, val);
359359
} else if (msicap_access(vdev, offset)) {
360360
(void)vmsi_write_cfg(vdev, offset, bytes, val);
@@ -371,7 +371,7 @@ static int32_t vpci_write_pt_dev_cfg(struct pci_vdev *vdev, uint32_t offset,
371371
static int32_t vpci_read_pt_dev_cfg(const struct pci_vdev *vdev, uint32_t offset,
372372
uint32_t bytes, uint32_t *val)
373373
{
374-
if (vbar_access(vdev, offset, bytes)) {
374+
if (vbar_access(vdev, offset)) {
375375
(void)vdev_pt_read_cfg(vdev, offset, bytes, val);
376376
} else if (msicap_access(vdev, offset)) {
377377
(void)vmsi_read_cfg(vdev, offset, bytes, val);

‎hypervisor/dm/vpci/vpci_priv.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,25 +104,25 @@ static inline bool msixcap_access(const struct pci_vdev *vdev, uint32_t offset)
104104
/**
105105
* @pre vdev != NULL
106106
*/
107-
static inline bool vbar_access(const struct pci_vdev *vdev, uint32_t offset, uint32_t bytes)
107+
static inline bool vbar_access(const struct pci_vdev *vdev, uint32_t offset)
108108
{
109-
return (is_bar_offset(vdev->nr_bars, offset) && (bytes == 4U) && ((offset & 0x3U) == 0U));
109+
return is_bar_offset(vdev->nr_bars, offset);
110110
}
111111

112112
/**
113113
* @pre vdev != NULL
114114
*/
115115
static inline bool has_msi_cap(const struct pci_vdev *vdev)
116116
{
117-
return (vdev->msi.capoff != 0U);
117+
return (vdev->msi.capoff != 0U);
118118
}
119119

120120
/**
121121
* @pre vdev != NULL
122122
*/
123123
static inline bool msicap_access(const struct pci_vdev *vdev, uint32_t offset)
124124
{
125-
return (has_msi_cap(vdev) && in_range(offset, vdev->msi.capoff, vdev->msi.caplen));
125+
return (has_msi_cap(vdev) && in_range(offset, vdev->msi.capoff, vdev->msi.caplen));
126126
}
127127

128128
/**

0 commit comments

Comments
 (0)