Message ID | 54C8A2AC.9020203@ozlabs.ru |
---|---|
State | New |
Headers | show |
On Wed, Jan 28, 2015 at 07:49:48PM +1100, Alexey Kardashevskiy wrote: > On 01/28/2015 08:21 AM, Alexey Kardashevskiy wrote: > > On 01/27/2015 05:13 PM, Alexey Kardashevskiy wrote: [snip] > >> diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h > >> index 46edc2a..6ad55d1 100644 > >> --- a/include/hw/ppc/spapr_vio.h > >> +++ b/include/hw/ppc/spapr_vio.h > >> @@ -64,6 +64,8 @@ struct VIOsPAPRDevice { > >> target_ulong signal_state; > >> VIOsPAPR_CRQ crq; > >> AddressSpace as; > >> + MemoryRegion mrroot; > >> + MemoryRegion mrbypass; > >> sPAPRTCETable *tcet; > >> }; > >> > >> > > > > > > > I believe doing something like this is way too disguising because of > tobj->parent? It's kinda ugly, but it's the best way I can think of. So, I think this is a reasonable approach, but it does need a comment saying why the hack is necessary. > > > > --- a/hw/ppc/spapr_iommu.c > +++ b/hw/ppc/spapr_iommu.c > @@ -25,6 +25,7 @@ > #include "trace.h" > > #include "hw/ppc/spapr.h" > +#include "hw/ppc/spapr_vio.h" > > #include <libfdt.h> > > @@ -88,10 +89,26 @@ static IOMMUTLBEntry > spapr_tce_translate_iommu(MemoryRegion *iommu, hwaddr addr, > return ret; > } > > +static int spapr_tce_table_post_load(void *opaque, int version_id) > +{ > + sPAPRTCETable *tcet = SPAPR_TCE_TABLE(opaque); > + Object *tobj = OBJECT(tcet); > + Object *vobj = object_dynamic_cast(tobj->parent, TYPE_VIO_SPAPR_DEVICE); > + > + if (!vobj) { > + return 0; > + } > + > + spapr_vio_set_bypass((VIOsPAPRDevice *) vobj, tcet->bypass); > + > + return 0; > +} > + > static const VMStateDescription vmstate_spapr_tce_table = { > .name = "spapr_iommu", > .version_id = 2, > .minimum_version_id = 2, > + .post_load = spapr_tce_table_post_load, > .fields = (VMStateField []) { > /* Sanity check */ > VMSTATE_UINT32_EQUAL(liobn, sPAPRTCETable), > > >
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 01/29/2015 11:31 AM, David Gibson wrote: > On Wed, Jan 28, 2015 at 07:49:48PM +1100, Alexey Kardashevskiy wrote: >> On 01/28/2015 08:21 AM, Alexey Kardashevskiy wrote: >>> On 01/27/2015 05:13 PM, Alexey Kardashevskiy wrote: > [snip] >>>> diff --git a/include/hw/ppc/spapr_vio.h >>>> b/include/hw/ppc/spapr_vio.h index 46edc2a..6ad55d1 100644 --- >>>> a/include/hw/ppc/spapr_vio.h +++ b/include/hw/ppc/spapr_vio.h @@ >>>> -64,6 +64,8 @@ struct VIOsPAPRDevice { target_ulong >>>> signal_state; VIOsPAPR_CRQ crq; AddressSpace as; + >>>> MemoryRegion mrroot; + MemoryRegion mrbypass; sPAPRTCETable >>>> *tcet; }; >>>> >>>> >>> >>> >> >> >> I believe doing something like this is way too disguising because >> of tobj->parent? > > It's kinda ugly, but it's the best way I can think of. Better than having VIOsPAPRDevice pointer in sPAPRTCETable? I would expect a object_get_parent() helper to exist but it is not there and I believe this is for a reason (may be it will be get rid of some time later)... > So, I think this is a reasonable approach, but it does need a comment > saying why the hack is necessary. > >> >> >> >> --- a/hw/ppc/spapr_iommu.c +++ b/hw/ppc/spapr_iommu.c @@ -25,6 +25,7 >> @@ #include "trace.h" >> >> #include "hw/ppc/spapr.h" +#include "hw/ppc/spapr_vio.h" >> >> #include <libfdt.h> >> >> @@ -88,10 +89,26 @@ static IOMMUTLBEntry >> spapr_tce_translate_iommu(MemoryRegion *iommu, hwaddr addr, return >> ret; } >> >> +static int spapr_tce_table_post_load(void *opaque, int version_id) >> +{ + sPAPRTCETable *tcet = SPAPR_TCE_TABLE(opaque); + Object >> *tobj = OBJECT(tcet); + Object *vobj = >> object_dynamic_cast(tobj->parent, TYPE_VIO_SPAPR_DEVICE); + + if >> (!vobj) { + return 0; + } + + >> spapr_vio_set_bypass((VIOsPAPRDevice *) vobj, tcet->bypass); + + >> return 0; +} + static const VMStateDescription >> vmstate_spapr_tce_table = { .name = "spapr_iommu", .version_id = 2, >> .minimum_version_id = 2, + .post_load = >> spapr_tce_table_post_load, .fields = (VMStateField []) { /* >> Sanity check */ VMSTATE_UINT32_EQUAL(liobn, sPAPRTCETable), >> >> >> > - -- Alexey -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJUyYNtAAoJEIYTPdgrwSC54t0P/0SGpr+UX7qlwKKs89bwVc// dzUS4wPXeH7zYkLughLprEh3uINY9x7z8euqSJg1wPoh54F6QX9PDb8ipKavie+a NlxDeqVNh+eWLR/rI1OxMbg/PcQSnqCN13EHJLiz69xkdEZsNReLKUFueMrk4Grl H4l3+GDVx6HbBSTWawOmcpN4Bcz0/nRgsYf6RytIk9ZP60zplbNkrCt3jwPV87O3 /+g73eBvpusiY+8NIEPbzAEssMcTj+JDYB5Dy5JlsOYewQFyXiiXaVSdRnNIPB4o tSaD93LYvkNEYXACJ7awsfWWx45tvWjt0GtO0YUU2vGD/cAWjsW8Q76f1DjNZdvD wZwhJF0cieaRnlrNMlCE199DyxcKDqDsENEnRUsySE8qq1uHGXo1dlyn+rA3Spc7 jEfwxkmtpHf4l9JAB01hGmq37eYPPKS9uyjXwqtKZuDea0ObVEtsmXiSZnOho82D +jJzr1KUDUMFcWw7ZWxXJ4I1wCt0u3krRWtO4vD0RXnjWgKUM8MbuNGFBFUt1qtF 8M1KTrm06jW4w0h19nBSP2ed7j2ObE2KJebE7I/tl//gxgQo4PmR/cwZv8EXpHCC KNz6SDvUSboJ9i02Gxkk7GWKYPbGthQkrICxTfVwKGyms4O58/KiH8T/Koey/BLJ 92imwmKo3VtddbQfe5Ay =dlFh -----END PGP SIGNATURE-----
On Thu, Jan 29, 2015 at 11:48:46AM +1100, Alexey Kardashevskiy wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On 01/29/2015 11:31 AM, David Gibson wrote: > > On Wed, Jan 28, 2015 at 07:49:48PM +1100, Alexey Kardashevskiy wrote: > >> On 01/28/2015 08:21 AM, Alexey Kardashevskiy wrote: > >>> On 01/27/2015 05:13 PM, Alexey Kardashevskiy wrote: > > [snip] > >>>> diff --git a/include/hw/ppc/spapr_vio.h > >>>> b/include/hw/ppc/spapr_vio.h index 46edc2a..6ad55d1 100644 --- > >>>> a/include/hw/ppc/spapr_vio.h +++ b/include/hw/ppc/spapr_vio.h @@ > >>>> -64,6 +64,8 @@ struct VIOsPAPRDevice { target_ulong > >>>> signal_state; VIOsPAPR_CRQ crq; AddressSpace as; + > >>>> MemoryRegion mrroot; + MemoryRegion mrbypass; sPAPRTCETable > >>>> *tcet; }; > >>>> > >>>> > >>> > >>> > >> > >> > >> I believe doing something like this is way too disguising because > >> of tobj->parent? > > > > It's kinda ugly, but it's the best way I can think of. > > > > Better than having VIOsPAPRDevice pointer in sPAPRTCETable? I would > expect a object_get_parent() helper to exist but it is not there and I > believe this is for a reason (may be it will be get rid of some time > later)... Uh.. I think someone who knows qom better than me will have to answer that..
--- a/hw/ppc/spapr_iommu.c +++ b/hw/ppc/spapr_iommu.c @@ -25,6 +25,7 @@ #include "trace.h" #include "hw/ppc/spapr.h" +#include "hw/ppc/spapr_vio.h" #include <libfdt.h> @@ -88,10 +89,26 @@ static IOMMUTLBEntry spapr_tce_translate_iommu(MemoryRegion *iommu, hwaddr addr, return ret; } +static int spapr_tce_table_post_load(void *opaque, int version_id) +{ + sPAPRTCETable *tcet = SPAPR_TCE_TABLE(opaque); + Object *tobj = OBJECT(tcet); + Object *vobj = object_dynamic_cast(tobj->parent, TYPE_VIO_SPAPR_DEVICE); + + if (!vobj) { + return 0; + } + + spapr_vio_set_bypass((VIOsPAPRDevice *) vobj, tcet->bypass); + + return 0; +} + static const VMStateDescription vmstate_spapr_tce_table = { .name = "spapr_iommu", .version_id = 2, .minimum_version_id = 2, + .post_load = spapr_tce_table_post_load, .fields = (VMStateField []) { /* Sanity check */