[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH v6 08/12] pcie/aer: helper functions for pcie ae
From: |
Michael S. Tsirkin |
Subject: |
[Qemu-devel] Re: [PATCH v6 08/12] pcie/aer: helper functions for pcie aer capability |
Date: |
Fri, 22 Oct 2010 11:27:23 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Fri, Oct 22, 2010 at 08:53:15AM +0900, Isaku Yamahata wrote:
> On Thu, Oct 21, 2010 at 10:07:01AM +0200, Michael S. Tsirkin wrote:
> > On Thu, Oct 21, 2010 at 02:15:24PM +0900, Isaku Yamahata wrote:
> > > Thank you for detailed review.
> > >
> > > On Wed, Oct 20, 2010 at 11:56:16AM +0200, Michael S. Tsirkin wrote:
> > > > > +static uint32_t aer_log_del(PCIEAERLog *aer_log)
> > > > > +{
> > > > > + uint32_t i = aer_log->consumer;
> > > > > + aer_log->consumer = aer_log_next(aer_log->consumer,
> > > > > aer_log->log_max);
> > > > > + return i;
> > > > > +}
> > > >
> > > >
> > > > Please just use 'int' or 'unsigned' instead of uint32_t if you simply
> > > > want to say 'a number'. Using specific length makes it impossible to
> > > > say where you *really* want a value.
> > >
> > > PCIEAERLog is saved/loaded. So explicit sized number is chosen.
> >
> > I didn't notice. But consumer/producer are not guest visible, are they?
> > If yes I think it's a mistake to save/load them, tying us to
> > a specific implementation: just calculate and save the # of
> > valid entries in the log.
>
> ACIEAERLog and PCIEAERErr themselves are the implementation specific
> internal states which is not visible to guest.
> So there is no point of arguing that consumer/producer are
> specific to implementation.
Well the errors can be read out by the guest, so they are
potentially guest visible. Thus I think the only thing
we want to migrate is the list of errors logged.
> > Also I put the comment here but it is a general one: pls go over the
> > code, and just take a look at what types you use all over and think
> > whether size really matters. In most places it does not, it just must be
> > big enough, so use int or unsigned there. It will be much harder for
> > others to do so later.
>
> Will do.
> --
> yamahata
[Qemu-devel] [PATCH v6 05/12] x3130: pcie upstream port, Isaku Yamahata, 2010/10/20
[Qemu-devel] [PATCH v6 09/12] pcie/aer: glue aer error injection into qemu monitor, Isaku Yamahata, 2010/10/20
[Qemu-devel] [PATCH v6 02/12] pci/bridge: fix pci_bridge_reset(), Isaku Yamahata, 2010/10/20
[Qemu-devel] [PATCH v6 07/12] pcie/hotplug: introduce pushing attention button command, Isaku Yamahata, 2010/10/20