From: Pavel Fedin <p.fedin@samsung.com>
To: "'Michael S. Tsirkin'" <mst@redhat.com>
Cc: 'Paolo Bonzini' <pbonzini@redhat.com>,
qemu-devel@nongnu.org, 'Peter Maydell' <peter.maydell@linaro.org>
Subject: Re: [Qemu-devel] [PATCH v4 2/3] hw/pci: Introduce pci_requester_id()
Date: Thu, 15 Oct 2015 11:54:57 +0300 [thread overview]
Message-ID: <00c201d10727$2b9921f0$82cb65d0$@samsung.com> (raw)
In-Reply-To: <20151014190920-mutt-send-email-mst@redhat.com>
Hello!
> > diff --git a/stubs/pci.c b/stubs/pci.c
> > new file mode 100644
> > index 0000000..3b13000
> > --- /dev/null
> > +++ b/stubs/pci.c
> > @@ -0,0 +1,16 @@
> > +/*
> > + * QEMU architecture-specific PCI functions
> > + *
> > + * Copyright (c) 2015 Samsung Electronics Co., Ltd.
> > + * Written by Pavel Fedin
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + *
> > + */
> > +#include "hw/pci/pci.h"
> > +
> > +uint16_t pci_requester_id(PCIDevice *dev)
> > +{
> > + return 0;
> > +}
>
> OK this is now wrong. You should move the logic back here
> from target-arm.
Thank you very much for quick responses and cooperation, but... Could we discuss a little bit, what you actually want as a result?
This starts to be very inefficient and time-consuming. Every time i redo the whole thing only to hear that "this single line is now
wrong".
So far, previously you told me that since requester ID is architecture-specific, if should be moved to arch code. Now you tell me
that you want it in stubs... Why? Stubs are something to be replaced. And we won't have the replacement anywhere, because nowadays
no architecture except ARM uses it. And probably it will stay this way forever. The function is very small, and even if we add more
bits for root complex ID, it will stay very small. Why not to have it as inline in include/hw/pci.h? If we ever need to replace it
in future, then we'll move it to stubs and override in target-XXX.
And, what with other two patches? Actually, all three are independent, i combined them into one series only because they are part
of original bigger series. You didn't have any comments on them, could you ACK them finally in order to simplify things down?
Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
next prev parent reply other threads:[~2015-10-15 8:55 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-14 15:06 [Qemu-devel] [PATCH v4 0/3] Make KVM/MSI code device-ID-aware Pavel Fedin
2015-10-14 15:06 ` [Qemu-devel] [PATCH v4 1/3] kvm: Make KVM_CAP_SIGNAL_MSI globally available Pavel Fedin
2015-10-14 15:06 ` [Qemu-devel] [PATCH v4 2/3] hw/pci: Introduce pci_requester_id() Pavel Fedin
2015-10-14 16:09 ` Michael S. Tsirkin
2015-10-15 8:54 ` Pavel Fedin [this message]
2015-10-15 9:36 ` Michael S. Tsirkin
2015-10-15 9:42 ` Pavel Fedin
2015-10-15 9:58 ` Michael S. Tsirkin
2015-10-14 15:06 ` [Qemu-devel] [PATCH v4 3/3] kvm: Pass PCI device pointer to MSI routing functions Pavel Fedin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='00c201d10727$2b9921f0$82cb65d0$@samsung.com' \
--to=p.fedin@samsung.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).