* [Qemu-devel] [PATCH RFC] msi: remove range checks
@ 2010-10-25 6:07 Michael S. Tsirkin
2010-10-25 7:07 ` [Qemu-devel] " Isaku Yamahata
0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2010-10-25 6:07 UTC (permalink / raw)
To: qemu-devel, yamahata
config write handlers should be idempotent.
So no need for range checks.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/msi.c | 29 -----------------------------
1 files changed, 0 insertions(+), 29 deletions(-)
diff --git a/hw/msi.c b/hw/msi.c
index a949d82..af8ea44 100644
--- a/hw/msi.c
+++ b/hw/msi.c
@@ -257,35 +257,6 @@ void msi_write_config(PCIDevice *dev, uint32_t addr, uint32_t val, int len)
uint32_t pending;
int i;
-#ifdef MSI_DEBUG
- if (ranges_overlap(addr, len, dev->msi_cap, msi_cap_sizeof(flags))) {
- MSI_DEV_PRINTF(dev, "addr 0x%"PRIx32" val 0x%"PRIx32" len %d\n",
- addr, val, len);
- MSI_DEV_PRINTF(dev, "ctrl: 0x%"PRIx16" address: 0x%"PRIx32,
- flags,
- pci_get_long(dev->config + msi_address_lo_off(dev)));
- if (msi64bit) {
- fprintf(stderr, " addrss-hi: 0x%"PRIx32,
- pci_get_long(dev->config + msi_address_hi_off(dev)));
- }
- fprintf(stderr, " data: 0x%"PRIx16,
- pci_get_word(dev->config + msi_data_off(dev, msi64bit)));
- if (flags & PCI_MSI_FLAGS_MASKBIT) {
- fprintf(stderr, " mask 0x%"PRIx32" pending 0x%"PRIx32,
- pci_get_long(dev->config + msi_mask_off(dev, msi64bit)),
- pci_get_long(dev->config + msi_pending_off(dev, msi64bit)));
- }
- fprintf(stderr, "\n");
- }
-#endif
-
- /* Are we modified? */
- if (!(ranges_overlap(addr, len, msi_flags_off(dev), 2) ||
- (msi_per_vector_mask &&
- ranges_overlap(addr, len, msi_mask_off(dev, msi64bit), 4)))) {
- return;
- }
-
if (!(flags & PCI_MSI_FLAGS_ENABLE)) {
return;
}
--
1.7.3-rc1
^ permalink raw reply related [flat|nested] 3+ messages in thread* [Qemu-devel] Re: [PATCH RFC] msi: remove range checks
2010-10-25 6:07 [Qemu-devel] [PATCH RFC] msi: remove range checks Michael S. Tsirkin
@ 2010-10-25 7:07 ` Isaku Yamahata
2010-10-25 7:29 ` Michael S. Tsirkin
0 siblings, 1 reply; 3+ messages in thread
From: Isaku Yamahata @ 2010-10-25 7:07 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel
Seems good.
For consistency, the range check in msix_write_config() should
be also removed.
On Mon, Oct 25, 2010 at 08:07:25AM +0200, Michael S. Tsirkin wrote:
> config write handlers should be idempotent.
> So no need for range checks.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> hw/msi.c | 29 -----------------------------
> 1 files changed, 0 insertions(+), 29 deletions(-)
>
> diff --git a/hw/msi.c b/hw/msi.c
> index a949d82..af8ea44 100644
> --- a/hw/msi.c
> +++ b/hw/msi.c
> @@ -257,35 +257,6 @@ void msi_write_config(PCIDevice *dev, uint32_t addr, uint32_t val, int len)
> uint32_t pending;
> int i;
>
> -#ifdef MSI_DEBUG
> - if (ranges_overlap(addr, len, dev->msi_cap, msi_cap_sizeof(flags))) {
> - MSI_DEV_PRINTF(dev, "addr 0x%"PRIx32" val 0x%"PRIx32" len %d\n",
> - addr, val, len);
> - MSI_DEV_PRINTF(dev, "ctrl: 0x%"PRIx16" address: 0x%"PRIx32,
> - flags,
> - pci_get_long(dev->config + msi_address_lo_off(dev)));
> - if (msi64bit) {
> - fprintf(stderr, " addrss-hi: 0x%"PRIx32,
> - pci_get_long(dev->config + msi_address_hi_off(dev)));
> - }
> - fprintf(stderr, " data: 0x%"PRIx16,
> - pci_get_word(dev->config + msi_data_off(dev, msi64bit)));
> - if (flags & PCI_MSI_FLAGS_MASKBIT) {
> - fprintf(stderr, " mask 0x%"PRIx32" pending 0x%"PRIx32,
> - pci_get_long(dev->config + msi_mask_off(dev, msi64bit)),
> - pci_get_long(dev->config + msi_pending_off(dev, msi64bit)));
> - }
> - fprintf(stderr, "\n");
> - }
> -#endif
> -
> - /* Are we modified? */
> - if (!(ranges_overlap(addr, len, msi_flags_off(dev), 2) ||
> - (msi_per_vector_mask &&
> - ranges_overlap(addr, len, msi_mask_off(dev, msi64bit), 4)))) {
> - return;
> - }
> -
> if (!(flags & PCI_MSI_FLAGS_ENABLE)) {
> return;
> }
> --
> 1.7.3-rc1
>
--
yamahata
^ permalink raw reply [flat|nested] 3+ messages in thread* [Qemu-devel] Re: [PATCH RFC] msi: remove range checks
2010-10-25 7:07 ` [Qemu-devel] " Isaku Yamahata
@ 2010-10-25 7:29 ` Michael S. Tsirkin
0 siblings, 0 replies; 3+ messages in thread
From: Michael S. Tsirkin @ 2010-10-25 7:29 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: qemu-devel
Hmm, does this simply check that write is within the capability?
Then I think I misunderstood what this does.
Please replace hard-coded length logic with existing function call
that gets the lengths, the range check can stay then.
On Mon, Oct 25, 2010 at 04:07:13PM +0900, Isaku Yamahata wrote:
> Seems good.
> For consistency, the range check in msix_write_config() should
> be also removed.
>
> On Mon, Oct 25, 2010 at 08:07:25AM +0200, Michael S. Tsirkin wrote:
> > config write handlers should be idempotent.
> > So no need for range checks.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > hw/msi.c | 29 -----------------------------
> > 1 files changed, 0 insertions(+), 29 deletions(-)
> >
> > diff --git a/hw/msi.c b/hw/msi.c
> > index a949d82..af8ea44 100644
> > --- a/hw/msi.c
> > +++ b/hw/msi.c
> > @@ -257,35 +257,6 @@ void msi_write_config(PCIDevice *dev, uint32_t addr, uint32_t val, int len)
> > uint32_t pending;
> > int i;
> >
> > -#ifdef MSI_DEBUG
> > - if (ranges_overlap(addr, len, dev->msi_cap, msi_cap_sizeof(flags))) {
> > - MSI_DEV_PRINTF(dev, "addr 0x%"PRIx32" val 0x%"PRIx32" len %d\n",
> > - addr, val, len);
> > - MSI_DEV_PRINTF(dev, "ctrl: 0x%"PRIx16" address: 0x%"PRIx32,
> > - flags,
> > - pci_get_long(dev->config + msi_address_lo_off(dev)));
> > - if (msi64bit) {
> > - fprintf(stderr, " addrss-hi: 0x%"PRIx32,
> > - pci_get_long(dev->config + msi_address_hi_off(dev)));
> > - }
> > - fprintf(stderr, " data: 0x%"PRIx16,
> > - pci_get_word(dev->config + msi_data_off(dev, msi64bit)));
> > - if (flags & PCI_MSI_FLAGS_MASKBIT) {
> > - fprintf(stderr, " mask 0x%"PRIx32" pending 0x%"PRIx32,
> > - pci_get_long(dev->config + msi_mask_off(dev, msi64bit)),
> > - pci_get_long(dev->config + msi_pending_off(dev, msi64bit)));
> > - }
> > - fprintf(stderr, "\n");
> > - }
> > -#endif
> > -
> > - /* Are we modified? */
> > - if (!(ranges_overlap(addr, len, msi_flags_off(dev), 2) ||
> > - (msi_per_vector_mask &&
> > - ranges_overlap(addr, len, msi_mask_off(dev, msi64bit), 4)))) {
> > - return;
> > - }
> > -
> > if (!(flags & PCI_MSI_FLAGS_ENABLE)) {
> > return;
> > }
> > --
> > 1.7.3-rc1
> >
>
> --
> yamahata
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-10-25 7:36 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-25 6:07 [Qemu-devel] [PATCH RFC] msi: remove range checks Michael S. Tsirkin
2010-10-25 7:07 ` [Qemu-devel] " Isaku Yamahata
2010-10-25 7:29 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).